Write-only variables considered harmful? Or beneficial?

Write-only variables considered harmful? Or beneficial?

Rate This
  • Comments 16

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.

  • 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 -

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

  • There are a few patterns I find myself using that will eventually be optimized out, but I want them in simply for the debugger.  

    The write-only-variable is one, and now I have a convenient name for it.

    Another is the no-op for breakpoint (no language in particular).  This actually combines the write-only-variable and the no-op:

    void foo(...)

    {

       int i;             // Just to get the return value

       i = Func();  

       return;         // <--  Unnecessary, just here to give me a breakpoint so I can see i

    }

    Simply because source debuggers' behavior differs *wildly* about how to trap-when-leaving-scope.  For GUI's, do I select the } as a breakpoint?  Will my local variables be cleaned up by then or no?

    Also seen with loops that are full of blocks, and no convenient place to latch onto after the blocks are run, but still at a known place in the loop:

    while(test)

    {

       if (...)  { }

       switch(...) { ... }

      continue;            // <--- I want to stop at the bottom of each and every loop, break here.

    }

    Of course, in assembler, there's perfectly obvious places to break but given that I can't see the RTS or the BNE in the source debugger, this clutter happens.  (And is later removed.  If I remember.)

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

  • #if DEBUG

    bool x = DoSomething();

    #else

    DoSomething();

    #endif

  • (The idea in the previous post was that debug builds are expected to have warnings, where release code should hypothetically be warning-free)

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

    And of course, the VC++ debugger *did* show me just that - which made that nested function call example a very easy thing to watch. 'Till reading this post, i'd assumed that the C# debugger also supported it, and that i'd just screwed up my installation horribly in some improbable way... thanks for simultaneously breaking my heart and restoring my confidence in my own sanity.

  • 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?

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

  • Never, never, never (what never? hardly ever) put functional code inside of #ifdef DEBUG sections!

    It's a really bad habit to get into, and it leads to things like:

    #if DEBUG

    bool x = Divide(1,1);

    #else

    Divide(1,0);

    #endif

    "It always works in the debug version, but crashes every time in the release build!"

    As for the previous notes about adding "return" or "continue" into a function or loop to allow a breakpoint at the end of the function or as the loop is about to loop, I seem to remember that you can actually put a breakpoint on the line with the closing brace.

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

  • You are getting old and wise when you start to think about the poor sole working on the code later.  Thanks.

  • Since that "poor soul" is often oneself, it's not entirely altruistic!

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

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

Page 1 of 2 (16 items) 12