Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Which would be considered more maintainable?

if (a == b) c = true; else c = false;

or

 c = (a == b);

I've tried looking in Code Complete, but can't find an answer.

I think the first is more readable (you can literally read it out loud), which I also think makes it more maintainable. The second one certainly makes more sense and reduces code, but I'm not sure it's as maintainable for C# developers (I'd expect to see this idiom more in, for example, Python).

share|improve this question
3  
The two are not equivalent. You need a else c = false for the first or make the assignment an ||= in the second. –  delnan Nov 30 '12 at 21:31
13  
I think the fact that you've had to make two edits to the first form answers your question! –  James Nov 30 '12 at 21:38
7  
I agree with @James. The second form, while not as verbose, is very straightforward to understand and leaves no ambiguity as to its meaning. There are no tricks or shortcuts being taken, it's just short because the concept is simple. The fact that you coded the first one with a bug, and have had to edit it to fix it, and it's still not perfect (inconsistent use of braces), is proof positive that it's not as straightforward as you think. –  Eric King Nov 30 '12 at 21:46
5  
Wow, do C# devs really consider the first form acceptable? This... seriously reduces my trust in them. In my experience, use of the first form heavily hints at a complete misunderstanding of boolean expressions. –  Andres F. Nov 30 '12 at 21:56
2  
Just to keep things interesting, here's another option: c = a==b ? true : false; –  FrustratedWithFormsDesigner Nov 30 '12 at 22:13
show 10 more comments

5 Answers 5

up vote 10 down vote accepted

The second option is better.

There's definite reason to be wary of clever programming shortcuts that hurt maintainability by obscuring the intent of the code. So, I don't blame you for asking the question.

However, I don't consider c = (a == b); to be an example of a clever trick. It's a straightforward representation of a simple concept. As straight-forward as you can get.

A proper, "maintainable" formatting of your first example (without the missing braces and one-line construct, which I do consider a clever shortcut) would yield this code:

if (a == b)
{
    c = true; 
}
else 
{
    c = false;
}

In my experience, writing simple boolean logic in such a verbose, error-prone way is a sign of "iffy" code. It would make me wonder how more complex logic is being handled in this code-base.

share|improve this answer
add comment

First, realize that your two forms are not equivalent.

if (a == b) c = true;

c will be set to true if a is equal to b, and if not, its value will remain whatever it already is.

c = (a == b);

c will be set to true if a is equal to b, and if not, it will be set to false.

If you want the equivalent of the second form, in the style of the first form, you need to write it like this:

if (a == b) {
  c = true;
} else c = false;

Now it's clear which of the two is more readable, more maintainable, and less likely to introduce bugs if something is changed. Stick with the second form.

share|improve this answer
    
Absolutely right -- I've updated my question. –  Bret Walker Nov 30 '12 at 21:38
    
I consider the second form overly long and complicated - iff you want that effect, use a boolean assignment. –  Martin Schröder Dec 1 '12 at 12:08
add comment

I'd disagree that your first form is more readable - it's certainly not idiomatic C# to have two statements on a single line, and it's not recommended to have an if statement without using braces.

Secondly, I don't see how the second form is less maintainable - there's nothing to maintain. It's a simple statement of the relationship between a and b and it couldn't be expressed any more simply.

Another reason to prefer the second form is that you can declare c and assign it in a single statement i.e.

bool c = (a == b);

Modifying variables can easily lead to errors, so I would avoid it. Using an if statement requires the variable to be declared before the conditional and then modified.

share|improve this answer
    
I read the two statements on one line as pseudo code –  Jimmy Hoffa Nov 30 '12 at 23:39
1  
+1 for "Another reason to prefer the second form is that you can declare c and assign it in a single statement" –  Andres F. Dec 1 '12 at 20:14
add comment

"more maintainable" could be very subjective.

I usually prefer readability and intent over code reduction. I think you save 8 typed characters by using the reduced form.

Taking the language and culture around the language is a characteristic of 'readability' in my opinion.

There are times when performance may be cause for reducing the code to optimize the resulting byte code, but that should be done carefully after some profiling.

share|improve this answer
    
Subjective: Duh. Code reduction: Can also (if not overdone) improve clarity and communicate intent better. Performance: Of no concern at all (optimization is sometimes vital, but this isn't something that falls under optimization, because it practically never matters -- probably not even if you're writing kernels or drivers). –  delnan Nov 30 '12 at 21:37
4  
The first form had at least 1 bug which needed to be corrected, hidden in its verbosity. Second form was simple and to the point, and had no bug. I rest my case. In any case, the main purpose was not to type less characters! And this question wasn't about performance at all, which is irrelevant in this case. –  Andres F. Nov 30 '12 at 22:02
    
@delnan exactly my point, Duh. someone's code reduction is someone else's clarity. I did not cite optimization as the primary concern ... I used it as an example. Your reason for adding clever tricks or going outside of the norm should be balanced by whatever you/culture/enterprise will consider as readable. –  Johnnie Nov 30 '12 at 23:56
1  
Writing a clean boolean expression is not a clever trick! –  Andres F. Dec 1 '12 at 20:13
    
I guess I would put more merit on your comment if you actually addressed the spirit of the question and not an inane analogy used to prop up an underlying concept. –  Johnnie Dec 2 '12 at 1:11
add comment

Down-voters, please elaborate what is wrong with my revised answer.

Yes, c = (a == b); can be hard to read (worse yet, StyleCop complains about the unnecessary parenthesis), but I still like the simplicity of a == b. Therefore, here is what I like to use when both a and b are the same:

private static bool NoPeriod
{
    get
    {
        return this.MyWaveLength == this.HerWaveLength;
    }
}

And then you can do: this.c = this.NoPeriod instead of:

this.c = this.MyWavelength == this.HerWaveLength;
share|improve this answer
1  
so did you mean return this.MyWaveLength = this.HerWaveLength; or return this.MyWaveLength == this.HerWaveLength; instead? –  Jesse C. Slicer Nov 30 '12 at 21:57
5  
What!? c = (a == b); is not error-prone. First form in the original question is way more error prone, as demonstrated by the OP itself having to edit his/her question to fix bugs! –  Andres F. Nov 30 '12 at 21:59
    
I guess your suggested solution has a bug = vs. == –  Ondra Nov 30 '12 at 22:11
    
@Ondra Morský, thanks ... the compiler would have caught this for me. –  Job Nov 30 '12 at 22:20
5  
I'm not familiar with StyleCop, but if it tells you unnecessary parentheses are bad, you should turn it off. Unnecessary parentheses are not necessary for the compiler, but they can be very, very nice for human eyes. If you wrote c = a == b; the compiler can figure it out just fine, but it's harder for a human. –  Michael Shaw Dec 1 '12 at 0:02
show 1 more comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.