I have searched Programmers and Stackoverflow and was not able to come up with a
ID: 643264 • Letter: I
Question
I have searched Programmers and Stackoverflow and was not able to come up with a satisfying answer, even though I'm quite sure it must have been asked many times before. The only question I found has answers only dealing with readability. The code excerpts below are from actual C# code I was writing.
When dealing with a local variable that has some calculated value when a condition is true but a default value otherwise, do I:
First set the value, then change the value when the condition is true?
short version = 1; // <-- set the value here.
if (separator != -1)
{
version = Int16.Parse(filename.Substring(separator + 1));
filename = filename.Substring(0, separator);
}
Or not set the value, and add an else when the condition is false?
short version;
if (separator != -1)
{
version = Int16.Parse(filename.Substring(separator + 1));
filename = filename.Substring(0, separator);
}
else
version = 1; // <-- set the value here.
Could a programmer have wrong expectations when confronted with either solution (e.g. when the if-clause is quite big and the else-clause is way down, out of view)? Do you have any experiences where the difference mattered? Is one particular style enforced at your company, and why? Are there any technical reasons why one would be preferred over the other? The first one sets a variable twice, but I'm not sure whether this would matter in modern-day optimizing compilers.
Explanation / Answer
There's a school of thought that says a variable should not come into scope until you can assign it a valid value. In other words, the variable should never contain either a wrong value or a garbage value. This prevents the possibility of, for example, a maintainer later putting a call after the variable declaration but before the variable is properly initialized.
In your short example, it's fairly obvious those statements are a group, but over time there is nothing preventing people from putting several unrelated statements in between.
That means both of your examples are wrong. The first way has a short period of time when it incorrectly holds the default value, and the second way has a short period of time when it incorrectly holds no value. The best solution from a readability and maintainability point of view is something like:
short version = getVersion(filename, separator);
This has the added bonus of conforming to the "functions should do exactly one thing" school of thought as well.