Skip to main content
GameDev.net gamedev.net
🔒 Locked

Substance over Style?

Started by TheOddMan Dec 9, 2007 at 5:53 AM 9 replies 2.1k views
Original Post
TheOddMan
TheOddMan
I was having a conversation in work the other day about style, and one of my workmates suggested that case statements can be made clearer. Instead of writing things like: if( Object.SomeFlagSet() ) { DoSomething(); } if( !Object.AnotherFlagSet() ) { DoSomethingElse(); } He would write: if( true == Object.SomeFlagSet() ) { DoSomething(); } if( false == Object.AnotherFlagSet() ) { DoSomethingElse(); } He argues that his version makes things more clearer to read, and I agree with him. I like code that is easy to read, however I'm worried about whether his style would add extra overhead? To me these statements are equivalent to: if( operator==( Object.SomeFlagSet(), true ) ) { DoSomething(); } if( operator==( Object.AnotherFlagSet(), false ) ) { DoSomethingElse(); } So wouldn't this lead to an extra temporary object and function call for each conditional statement? Or would this be compiled out by a decent compiler?
Antheus
Antheus
Compilers will generate identical code, so that's not the problem.

But in general, (x == true) and (x == false) comparisons are frowned upon since they are redundant.

Similar to:
bool foobar() {  if (x == true) {    return true;  } else {    return false;  }}bool condition = foobar();// equivalent statement:bool condition = x;


There are circumstances where increased verbosity might be required, but generally, comparing for true and false is merely redundant.

The (CONSTANT == VARIABLE) pattern comes from elsewhere, where it helps prevent certain problems. Applying it to boolean conditions is usually merely an after effect of too much boiler-plate coding - it doesn't really add clarity (usually, there can be exceptions).
Sc4Freak
Sc4Freak
Your first example is the same as your second example, i.e. if(x) just means that x is compared to true, by invoking the == operator.

So, to answer your question, they are all exactly the same. Both in function and performance. Besides, even if there was a difference you should always trust your compiler to make tiny micro-optimisations like these - it's a lot better at them than you are.
CodeLuggage
CodeLuggage
I to do my checking in this way:
(CONDITION == STATEMENT) to improve readability. At a glance I can easily see every check in my code, and I'm sure the compiler will optimize it whichever way I do it.
Student at NITH, Norway2nd year of Gameprogramming BachelordegreeC++ enthusiast
thedustbustr
thedustbustr
Above all else, write good, standards compliant, factored, self documenting code. This may contain verbosity such as descriptive names and factored trivial methods.

Excluding this verbosity, I write as short code as possible. I don't unnecessarily return in void functions, I don't generally put spaces between operators, I don't put braces around a single statement when looping or testing, I write if(ptr). I do write NULL instead of 0 because the standard tells me to.

About the only thing I add that is optional are indents (my ide does it for me), and parentheses, because I'm too lazy to memorize the order of operations beyond +-*/.

The main rationale behind this is that if we rely on cues that are not enforced by the standard, we may fall victim of a mis-cue. Better to do exactly what is required, and do not use anything that is not required. This includes distrusting comments.

A competent programmer doesn't need your help in figuring out what the condition means. An incompetent programmer will mess it up no matter how you write it.

And don't concern yourself with the clock cycle difference between if(ptr) and if(operator==(ptr, true)). Talk about a premature optimization...
TheUnbeliever
TheUnbeliever
Do you say "I'll get you a drink if it is true that you're thirsty" or "I'll get you a drink if you're thirsty"?

On the other hand, you do what your manual of style instructs you to do, I guess.
[TheUnbeliever]
mikeman
mikeman
Quote:

He argues that his version makes things more clearer to read, and I agree with him.


I don't. Verbosity doesn't always mean more readable code. I find the first version more understandable, since I'm just so used to it. The second version's comparisons are redudant, so I marginally spend more time to parse them. "If" statements need a boolean expression to work. If you already have a boolean you don't need to compare it to true/false to get yet another boolean. It's like writing "if (false==(myVar==42))" instead of "if (myVar!=42)". To me, the second version seems kind of stupid. But it's all a matter of preference really. However, if it's for work, then you should probably use whatever style is generally used there.
Oluseyi
Oluseyi
Quote:
Original post by thedustbustr
Excluding this verbosity, I write as short code as possible. I don't unnecessarily return in void functions, I don't generally put spaces between operators, I don't put braces around a single statement when looping or testing, I write if(ptr). I do write NULL instead of 0 because the standard tells me to.

Textual brevity is not the same thing as semantic brevity. Semantic brevity is about limiting the number of concepts required to understand a particular block of code. Eliminating whitespace between operators, for instance, just makes the code harder to read, but doesn't actually make it shorter from a code or semantics perspective.

Similarly, eliminating braces on conditionals is more error-prone, in that it is easy to add another statement at the same indent level which visually suggests that it is part of the block, but isn't due to the missing braces. I prefer
if(condition) statement;
over
if(condition)   statement;
in such situations, except where the statement is an empty one, in which case the preference is reversed.

But, really, these are petty issues, mostly only existent because of the primitiveness of our tools.

Readable code respects that the programmer is competent, but doesn't challenge his skills with pointless "can you parse this über-dense expression" tests.
TheOddMan
TheOddMan
Well, the post was more referring to Boolean variables and functions that return true or false, rather than if( true == ( 12 == var ) ) )...

Still, valid points all round. It seems you're leaning in favour of not using his style.

As for the point on premature optimisations - it's not necessarily a case of that. It's kind of like always having ++i in for loops as there's never a case when i++ would be better. So you might as well adopt it as a part of style.

In this case I was thinking that there would never be a case where if( true == SomeFunc() ) is better than if( SomeFunc() ) - in fact is possibly worse, so it's better to not use his style.

Thanks all!
Zahlman
Zahlman
Quote:
Original post by TheOddMan
I was having a conversation in work the other day about style, and one of my workmates suggested that case statements can be made clearer. Instead of writing things like:
[edited]if( it is raining ){    bring an umbrella;}if( not it is snowing ){    cancel skiing trip;}He would write:if( it is true that it is raining ){    bring an umbrella;}if( it is false that it is snowing ){    cancel skiing trip;}

He argues that his version makes things more clearer to read, and I agree with him.


I would beg to differ.

Quote:
I like code that is easy to read, however I'm worried about whether his style would add extra overhead?


It will not.

Quote:
To me these statements are equivalent to:

...


They are. Both your way and his.

Quote:
So wouldn't this lead to an extra temporary object and function call for each conditional statement? Or would this be compiled out by a decent compiler?


It would actually take some effort to write the compiler in a way that avoided optimizing it out.
Zahlman
Zahlman
Quote:
Original post by Oluseyi
Textual brevity is not the same thing as semantic brevity. Semantic brevity is about limiting the number of concepts required to understand a particular block of code. Eliminating whitespace between operators, for instance, just makes the code harder to read, but doesn't actually make it shorter from a code or semantics perspective.


QFE.

Quote:
I prefer
if(condition) statement;
over
if(condition)   statement;
in such situations, except where the statement is an empty one, in which case the preference is reversed.


I prefer to brace things explicitly for those :)

if (condition) { statement; }if (condition) {} // more obviously "do nothing" than a semicolon would be


I also wish Python's 'pass' were optional. I have found it to inhibit refactoring :(

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.