MyBB Community Forums

I was bored and decided to look through the MyBB source to see how they did things, and was confused by a couple of instances of (IMHO) inefficient code.

The code went something like this:

if($value >= constant)
{
  return true;
}
else
{
  return false;
}

Why is it done this way? Wouldn't it have been easier and more efficient to do this:

return ($value >= constant);

I admit I don't know PHP as well as the developers, and it may be some kind of security thing, or perhaps someone felt it "increased" readability, but if someone were to explain this to me it would help clear things up.

Thank you!

It's for readability

Okay, thank you. I personally disagree that it makes it more readable, but that's my opinion and the code works so I have no problem. I'm not one of those "my way or it's a piece of crap" people .

If .. else .. statements are more readable to the average joe then implying that parentheses return a boolean value.

Yes, but then again, why would the average joe be looking through the MyBB source unless they already know something about PHP, in which case they usually know that the statement would cause the function to return a boolean value.

I feel that the source for a big, complex project like MyBB would be the last place a sane user should learn PHP, and that the first place they should look is http://www.php.net/manual/en/tutorial.php, which they would probably get to after googling "php tutorial."

If you don't agree, perhaps a developer could include a comment with the new return statement:

// This causes the function to return true if the condition is true, and false if the condition is false.
// The parentheses are to visually separate the condition from the return statement and are not necessary.
return ($value >= constant);

This is, in my opinion, just as clear as the current code, and I feel is a good combination of readability and efficiency.

The average joe who perhaps wants to get into programming and creating plugins for MyBB?

The comments (though maybe not those exact comments) should enable anyone who doesn't understand it to be able to figure it out, while still teaching an efficient way to perform it. Explaining the code: that's what comments are for, right?

Thank you for taking your time to have this discussion with me. I think I'll stop now (unless you would like to continue) since I think I'm starting to go "my way or it's crap" which I would rather not do, especially seeing as MyBB is most certainly not "crap," knowing the complexity of creating such a piece of software.

And should we paste that comment next to every single sub expression enclosed in parentheses that returns a boolean value or simply make the code readable?

(2009-07-28, 12:34 AM)Firestryke31 Wrote: [ -> ]Thank you for taking your time to have this discussion with me. I think I'll stop now (unless you would like to continue) since I think I'm starting to go "my way or it's crap" which I would rather not do, especially seeing as MyBB is most certainly not "crap," knowing the complexity of creating such a piece of software.

It's not that we think your way is 'crap', Firestryke. Yes, it could be more 'efficient' to do it that way, but we'd have to comment practically everything that's beyond the basic level of coding so that [i]everyone[/i[ could understand it. For example, if we used ternary operators, we'd have to explain what they do at every single one we used. That means there would be more comments in MyBB than code.

Developers just stick with the Development Standards Ryan drills into us. And he beats us with sticks if we get it wrong...

(2009-07-27, 03:49 PM)Firestryke31 Wrote: [ -> ]I was bored and [...] was confused by a couple of instances of (IMHO) inefficient code.

Inefficient code... this? It's just a different way of writing the very same thing, making it more a question of style, than efficiency. The PHP compiler could probably even optimize it to result in the same byte code either way you write it (although it probably won't do that, since compiling on the fly leaves very little time for compiler optimization).

Anyway, rather than doing guesswork, why not benchmark it?

Start an interactive PHP session (php -a) and enter this code:

function a() { if($value > 0) return true; else return false; }
function b() { return ($value > 0); }

Then call either function a million times:

for($value = 1000000; $value--; ) a();
for($value = 1000000; $value--; ) b();

Here's my results in seconds (running each test a 100 times):

a: min 0.518, max 0.685, avg 0.550
b: min 0.514, max 0.619, avg 0.544

Can you tell the difference?
Can you measure the difference?
Is the change worthwhile for an average 0.006 seconds difference for a million calls?