Tuesday, May 31, 2011

bad coding practice: variable assignment inside if

I've seen some programmers like to put variable assignment inside an if conditional statement:

if ($record = $someObject->getRecord()) {
//do something with $record
}
//maybe do something with $record after the if block

To me, this is a very bad practice. I cannot figure out any benefits of putting variable assignment inside an if conditional statement. Probably the only thing is you can save one line of code? But look at the code below:

$record = $someObject->getRecord();
if ($record){
//do something with $record
}
//maybe do something with $record after the if block

Which one is easier to read? Which one is easier to understand? Which one will NOT bring any possible confusion? How about we just TRY to do one task per line of code?

The idea of trying to write less code(and then do less work) is absolutely correct. But this has a prerequisite: we should firstly ensure we do the work properly. If one more line of code can make the logic clearer and eliminate confusion, then we should just write one more line.

I truly believe this statement: "Any fool can write code that a computer can understand. Good programmers write code that humans can understand". I think this statement comes from Martin Fowler's well known book "Refactoring: Improving the Design of Existing Code"? I believe this is a book that every programmer should keep reading often, particularly for those who work with languages that are very very flexible, like PHP.

No comments: