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