11

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).

15
  • 3
    The two are not equivalent. You need a else c = false for the first or make the assignment an ||= in the second.
    – user7043
    Nov 30, 2012 at 21:31
  • 17
    I think the fact that you've had to make two edits to the first form answers your question!
    – James
    Nov 30, 2012 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, 2012 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, 2012 at 21:56
  • 2
    Just to keep things interesting, here's another option: c = a==b ? true : false; Nov 30, 2012 at 22:13

6 Answers 6

15

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.

15

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.

2
  • Absolutely right -- I've updated my question. Nov 30, 2012 at 21:38
  • I consider the second form overly long and complicated - iff you want that effect, use a boolean assignment. Dec 1, 2012 at 12:08
11

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.

2
  • I read the two statements on one line as pseudo code Nov 30, 2012 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, 2012 at 20:14
0

"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.

5
  • 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).
    – user7043
    Nov 30, 2012 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, 2012 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, 2012 at 23:56
  • 1
    Writing a clean boolean expression is not a clever trick!
    – Andres F.
    Dec 1, 2012 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, 2012 at 1:11
0

The second. It has less repetition (DRY) and is easier to understand what is going on, that c holds to value of whether or not a and be b are equal.

IMHO, even better would be

c = a == b

Just as I would write

  • 1 + 2 + 3 instead of ((1 + 2) + 3)
  • 5 + 3 * 7 instead of (5 + (3 * 7))

Obviously and trivially unnecessary code is not a virtue. It's cluttered.

-4

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;
6
  • 1
    so did you mean return this.MyWaveLength = this.HerWaveLength; or return this.MyWaveLength == this.HerWaveLength; instead? Nov 30, 2012 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, 2012 at 21:59
  • I guess your suggested solution has a bug = vs. ==
    – Ondra
    Nov 30, 2012 at 22:11
  • @Ondra Morský, thanks ... the compiler would have caught this for me.
    – Job
    Nov 30, 2012 at 22:20
  • 6
    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. Dec 1, 2012 at 0:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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