DEV Community

loading...

Discussion on: Writing Clean Code

Collapse
gonedark profile image
Jason McCreary Author • Edited

I don't consider a single return with a compound condition readable. In general, simply having less lines doesn't improve code.

Collapse
mdor profile image
Marco Antonio Dominguez

At least for this example, is a good option use a single return statement, in case that we have more comparisons or complex ones would be better split on several statements.

The code is readable also I checked the code using these pages: phpcodechecker.com/ and piliapp.com/php-syntax-check/, it doesn't have issues.

Thread Thread
cyr1l profile image
cyr1l

Hi,
I did not find a php checker that indicates some bad practices in the code contrary to those of the javascript language (esprima.org/demo/validate.html, extendsclass.com/javascript-fiddle...).

Collapse
mariordev profile image
Mario

Exactly, packing conditionals into a single statement just to reduce the number of lines doesn't necessarily make the code more readable. I avoid compound conditions as much as possible. If I have to absolutely use them, I try to wrap them in a function with a name that better documents its purpose. Thanks for the great article.

Thread Thread
danidee10 profile image
Osaetin Daniel • Edited

I agree with you. I also avoid compound conditions as much as possible. They can easily introduce silent bugs into your code and there's the mental Overhead that comes with them When reading the code Compared to simple if Statements

Also the code with if statements is more maintainable and open to new features/improvements since each decision has its own block. For example you could easily add a Log message for each condition... Can't say the same for a single return statement.

Any programmer. (Even a newbie) can easily grok the if statements in one glance

Collapse
conaltuohy profile image
Conal Tuohy

I agree with Marco. To me "if (boolean-expression) return true" is a definite code smell. I would format the compound boolean expression so that each part has a line of its own though.

Thread Thread
gonedark profile image
Jason McCreary Author

That's interesting you consider it a code smell. I have some examples for future articles where I explore this raw boolean return statements.

In this case, I still wouldn't pack them down into one. At least not without some intermediate steps to improve the readability beyond just line breaks.

Thread Thread
algusdark profile image
Carlos.js

I agree with Marco, even we can do more functions:

function canView($scope, $ownerId) {
  return (hasScope('public') || isAdmin() || (hasScope('private') && isOwner($ownerId));
}

That way is easy to read. Obviously, that can be refactored.

Thread Thread
gonedark profile image
Jason McCreary Author

I'll explore this in a future post.

As a tangent, I never understood developers need to wrap the entire compound condition in parenthesis.

Thread Thread
kodikos profile image
Jodi Winters

I'd agree with carlos.js' solution, albeit each of the conditions being on separate lines. This supports the functions being just return statements, and giving clearer meaning (context) to the conditions you're checking. Also, it completely eliminates the branch logic in a function designed purely for information, a good nod to readable code.
The wrapping parenthesis must be a typo as there's no corresponding closing outer parenthesis. But that is an example of why you shouldn't wrap the condition in parentheses. There is another issue with Carlos.js' example, $scope is no longer used which means this must be a function in a class which means all those nice function calls are missing $this->

Thread Thread
joegaudet profile image
Joe Gaudet

100% agree that conditional returns of booleans is a smell, generally one of the first things I look for in code review.

Collapse
_andys8 profile image
Andy

In general concise code can be unreadable. But this example has the same content as the code in the blog post. There are more lines, brackets and boilerplate, but there is no additional information.

And I'd prefer getting rid of mixed cases, too. "Mix cases but be consistent" is not an option for me ;)

Collapse
ilmam profile image
ilmam

For me, I strongly agree with you, having 15 clean lines is much better than one condensed line. Consider someone new reading the both codebases: one look at the 15 lines will be enough to read and understand it. While they will struggle to evaluate all the booleans in their heads in the second option. Most probably will take longer to grasp.