Write-only variables considered harmful? Or beneficial?
I haven't forgotten that I promised to describe one more place where we insert an explicit conversion automatically. But before that, a quick digression.
Reader Phil Haack has written some good tips for writing clear code and quotes an old post of mine. However I thought that I would talk a bit about why it is sometimes a good idea to violate his last tip, which is basically "eschew write-only locals". His example is:
public void SomeMethod2()
{
bool x = DoSomething();
//some other code.
//...
//end of method. x is never used
}
Normally C# warns on all variables and fields which are never read, never written, etc. But in this case we suppress the warning on purpose if the assignment is not a constant expression.
This is because there is no good way in the Visual Studio debugger to say "show me the return value of the last function call". Though I would agree were you to sensibly point out that the way to solve this is to fix the debugger, given that I have no ability to fix it, we need a solution in C# for our customers.
Therefore we allow this assignment without producing a warning, and you can then very easily step through the debugger and see what functions returned what values when by looking at the locals window. If you build with optimizations turned on then of course the local is never generated in the IL.
This calls out a factor in code writing which is often glossed over. We ought to all know by now that the most important audience to consider when writing production code is not the compiler, but rather future maintenance programmers who will have to understand it. But we often forget that they don't come to understand it by simply reading the code; most of the time that I have to learn a piece of unfamiliar code in a big hurry I am looking at it live, running in a debugger. If the code works well with the debugger then it will be much easier to grasp.
As with all design problems, there are tradeoffs. Say we're building up an object which represents a mathematical expression. Which is better?
var add =
Add(
Multiply(
Constant(2),
Constant(3)),
Constant(4));
or
var c2 = Constant(2);
var c3 = Constant(3);
var mult = Multiply(c2, c3);
var c4 = Constant(4);
var add = Add(mult, c4);
Both have essentially the same number of lines of code. The optimizing compiler will generate exactly the same IL for both. But the former lexically emphasizes the structure of the resulting object, whereas the latter lexically emphasizes the order in which things actually get done. Moreover, the latter is much easier to debug if you want to know what the result of each stage along the way is.
Historically I have tended to favour the first pattern, but I am coming around to the second more and more these days in my own code. I find the former more elegant but harder to debug, and I spend a lot of time debugging. So Phil, don't go off on people who do this in C# too hard; they may be making your life easier when you have to debug something in the future.
Comments
Anonymous
April 23, 2007
Agree! Sometimes I see this in code I'm trying to understand: if (x==24) x = 24; By now I've learned that the last programmer needed to break when the variable reached a certain value, and did not want to figure out how to make the debugger do this for them. Of course, they should not have checked this in, but at least I know what they were trying to do -Anonymous
April 23, 2007
I understand your workaround but mostly agree with your point that the debugger should handle this. As people move towards functional programming styles the debugger needs the ability to understand units more granular than lines.Anonymous
April 23, 2007
The comment has been removedAnonymous
April 23, 2007
Hi Eric, I'm not sure I completely understand. I actually changed my example in my post to try and clarify my point. A better example is TryParse. For example, sometimes you want to parse a value and you don't care whether it succeeds. Here's an example: bool result; TryParse(someString, out result); I find it confusing if you do this: bool result; bool x = TryParse(someString, out result); And never use x afterwards. My point is that in this case, do you even care what value was returned by TryParse if you're never going to use that value? The only reason you called the method is because of the action that the method takes, not the value it returns. And I rarely go off on people. It's Resharper who is the hard taskmaster. I'm just the obsessive compulsive who tries to appease Resharper by trying to remove all its warnings. ;)Anonymous
April 23, 2007
#if DEBUG bool x = DoSomething(); #else DoSomething(); #endifAnonymous
April 23, 2007
(The idea in the previous post was that debug builds are expected to have warnings, where release code should hypothetically be warning-free)Anonymous
April 23, 2007
The comment has been removedAnonymous
April 23, 2007
So, then, would supporting the Command - Query Separation Principle by not writing or calling functions with the TrySomething pattern be desirable, in your opinion? If the example returned something more significant than a success flag in addition to the out parameter, would your reasoning regarding write-only variables to support debugging still stand?Anonymous
April 23, 2007
I know a few people who would definately answer that with: you should do test-driven development, not debugging. And I tend to agree with them more and more...Anonymous
April 23, 2007
The comment has been removedAnonymous
April 23, 2007
You probably can put breakpoints on the closing brace in Visual Studio. However, I spend my day in at least 4 programming environments each with different debuggers and I won't even relate how many different C IDE's I've put up with over the years... Some habits are more portable than others.Anonymous
April 24, 2007
You are getting old and wise when you start to think about the poor sole working on the code later. Thanks.Anonymous
April 24, 2007
Since that "poor soul" is often oneself, it's not entirely altruistic!Anonymous
March 03, 2011
I'm not particularly convinced. A warning doesn't prevent compilation, so if a dummy variable is added to aid debugging, it seems perfectly reasonable to issue a warning about it to remind the programmer to remove the unnecessary variable. And if they really want to leave debug code in the source forever, they can add an exclusion for the warning easily enough. I'd much rather have the compiler tell me that I forgot to do something with the variable I assigned than keep quiet, assuming that 1) I'm just adding it for debugging purposes, and 2) I intend to not remove it.Anonymous
March 06, 2011
Adam, that depends on your environment. In my environment, for example, warnings do prevent compilation, as we use the warnings as errors option. However, I do have the VC++ warning "variable initialized but never used" turned on for the reasons you describe. So in my VC++ environment, this is an error bool b = DoSomething(); but this is not bool b; b = DoSomething(); So VC++ does allow write-only variables, but warns about initialize-only variables.Anonymous
February 27, 2012
#if DEBUG bool b = #endif DoSomething(); // Apparently, this works in C# 4.