2012-01-13

The "Handoff" Pattern

I had been thinking about posting this for quite some time now, and all by coincidence I happened to get a chance to mention it just the other day in an answer that I wrote to a question on Programmers-StackExchange. So, here it is in a more formal way:

If class M stores or manipulates or in any other way works with instances of destructible (disposable) class D, it may not assume the responsibility to destruct these instances, unless it is explicitly told that ownership of these instances is transferred to it. Therefore, class M must accept a boolean called 'handoff' as a construction-time parameter, stating whether instances of D are being handed off to it, and it can therefore destruct them when it is done with them.

Example:
    //Note: the IReader interface extends IDisposable
    IReader reader = new BinaryStreamReader( ... );
    reader = new BufferedStreamReader( reader, handoff:true );
    try
    {
        /* use the reader interface */
    }
    finally
    {
        reader.Dispose(); //this destructs the buffered stream reader, and 
                          //destruction cascades to the binary stream
                          //reader because handoff was specified.
    }

Example:
    var collection = new CollectionOfDestructibles( handoff:true );
    collection.Add( new Destructible( 1 ) );
    collection.Add( new Destructible( 2 ) );
    collection.Add( new Destructible( 3 ) );
    collection.Dispose(); //this destructs the collection and every single
                          //one of its contents, since handoff was specified.

In languages which support optional parameters, the 'handoff' parameter should default to false.

2012-01-06

C# Blooper №3: No warnings about fields having already been initialized.


Before reading any further, please read the disclaimer.

When you declare a member variable and you pre-initialize it at the same time, and then you try to re-initialize it within the constructor without ever making use of its original pre-initialized value, you receive no warning about the field having already been initialized.

namespace Test3 
{  
    public class Test 
    {  
        public readonly string m = "m"; 
        public string n = "n"; 
        private string o = "o"; 
        protected readonly string p = "p"; 
        protected string q = "p"; 
        private string r = "r"; 

        Test() 
        {  
            m = "m2"; //Blooper: no warning about field having already been initialized. 
            n = "n2"; //Blooper: no warning about field having already been initialized. 
            o = "o2"; //Blooper: no warning about field having already been initialized. 
            p = "p2"; //Blooper: no warning about field having already been initialized. 
            q = "q2"; //Blooper: no warning about field having already been initialized. 
            r = "r2"; //Blooper: no warning about field having already been initialized. 
            o.ToLower(); //to prevent Warning CS0414: The field is assigned but its value is never used. 
            r.ToLower(); //to prevent Warning CS0414: The field is assigned but its value is never used. 
        }  
    } 
}  

This means that you may accidentally invoke complex initialization logic twice, unnecessarily wasting memory and clock cycles, and it may also lead to logic errors, if by any chance that initialization logic has side effects which are only meant to occur once. It may also confuse someone reading your code, (or even yourself looking at your code months later,) trying to figure out what's the purpose behind the seemingly repeated initialization, before the realization sinks in that it is simply redundant. Furthermore, if the re-initialization happens to differ from the pre-initialization, a good question arises, asking which one of the two was meant to be the correct one.

It is a pity, because the compiler could warn the programmer against this pitfall.

Also see related post: C# Blooper №2: No warnings about accessing uninitialized members.

-

C# Blooper №2: No warnings about accessing uninitialized members.


Before reading any further, please read the disclaimer.

When you declare a member variable, and then you try to read it from within the constructor without having first initialized it, you receive no warning about accessing an uninitialized member. This happens even if the member is declared as readonly.

namespace Test2  
{  
    public class Test 
    {  
        public readonly string m; 
        public string n; 
        protected readonly string o; 
        protected string p; 
        private readonly string q; 
        private string r; 

        Test() 
        {  
            m.ToUpper(); //Blooper: no warning about accessing uninitialized member. 
            n.ToUpper(); //Blooper: no warning about accessing uninitialized member. 
            o.ToUpper(); //Blooper: no warning about accessing uninitialized member. 
            p.ToUpper(); //Blooper: no warning about accessing uninitialized member. 
            q.ToUpper(); //Blooper: no warning about accessing uninitialized member. 
            r.ToUpper(); //Blooper: no warning about accessing uninitialized member. 
            q = "q"; //to prevent Warning CS0649: Field is never assigned to, and will always have its default value null 
            r = "r"; //to prevent Warning CS0649: Field is never assigned to, and will always have its default value null 
        }  
    } 
}  

Someone might argue that this is behavior is fine because the member in question is guaranteed to contain its default value. First of all, a readonly member containing its default value is completely useless. (See C# Blooper №1: No warnings about uninitialized readonly members when the class is public and the member is public, protected or protected internal.) Secondly, if the compiler is to help the developer catch potential errors and write better code, this is not a valid excuse: a different strategy is necessary.

If the programmer intends the member to contain its default value, then the programmer ought to explicitly state so. Failing to do so ought to imply intention to initialize the member later on, and certainly before any attempt is made to read the member.  This way, the programmer can have it both ways: they can have members pre-initialized to their default values, and they can receive warnings when they fail to initialize members.

Also please note that the compiler is capable of detecting that the value with which a member is being explicitly initialized is the default value for the type of the member, and so it can refrain from emitting any additional code for the assignment; thus, there is no performance issue.

Also see related post: C# Blooper №3: No warnings about fields having already been initialized.

-

2012-01-03

Pernicious Local Variable Initialization

Again and again I see programmers doing the following:
    string variable = string.Empty; 
    if( some.condition )
        variable = some.value;
    else
        variable = some.other.value;
You may see it with setting strings to String.Empty as in the example, or you may see it with setting integers to zero, pointers to null, etc. A very large number of programmers believe that when declaring a local variable you should always pre-initialize it with some value. The belief is so popular, that it practically enjoys "common knowledge" status, and is considered "best practice", despite being dead wrong. I call it The Practice of Pernicious Local Variable Initialization. Here is why.

The practice was not always bad. It started back in the dark ages of the first C compilers, when it was kind of a necessity. Compilers back then had a combination of unfortunate characteristics:
  1. They required all local variables in a function to be declared up-front.
  2. They did not require each variable to be initialized when declared. (A consequence of #1.)
  3. They were not smart enough to issue a warning if you tried to read a variable before writing it.
Back in those days, accidental use of uninitialized variables was a very common mistake, leading to many a monstrous bug. (See michael.gr - The Mother of All Bugs.) After having to troubleshoot and fix a few bugs of this kind in the first few days on the job, a new programmer would quickly learn to always pre-initialize every single local variable without asking why.

The practice of blindly pre-initializing variables continued well into the first half of the 1990s, even though by that time C and C++ compilers were capable of issuing warnings about uninitialized variables, because programmers were in the habit of either not enabling, or deliberately disabling, the warnings. Apparently, programmers would rather not have a piece of software suggest that it is smarter than themselves. (Besides, the pleasure of making undetected mistakes always was very popular, and still remains popular, judging by the universal trendiness of untyped scripting languages.)

After decades of blindly pre-initializing everything, it became a cargo cult habit, so programmers keep doing this today without really knowing why they are doing it, nor asking themselves if there are any downsides to this practice. 

And as it turns out, there are.

First of all, there is a violation of the principle of least surprise. When I see a variable being initialized to a certain value, I have to assume that this value has a certain role to play in the algorithm which follows. For example, seeing an integer being set to zero makes me think that what follows must be a loop using that integer as an accumulating sum; seeing a string variable being initialized with an empty string literal makes me expect a loop which will keep adding parts to it. So, when that's what I expect, it is rather disappointing when I look further down only to discover that none of that happens, and the value of the variable is just overwritten with something entirely different.

However, that's just a minor annoyance. 

It gets far worse than that.

By pre-initializing a variable with a value which is by definition meaningless, (since a meaningful value is not yet known at that time, otherwise you would just set that meaningful value to the variable and you would be done,) you are circumventing the safety checks of your compiler, and you are opening up the possibility of error: if you forget to assign a meaningful value to your variable further down, the compiler will not warn you, because as far as the compiler can tell, the variable has already received an initial value. The compiler does not know that the initial value is meaningless.

Luckily, modern compilers are not only capable of warning you if you attempt to use an uninitialized variable, but also warning you when you engage in Pernicious Local Variable Initialization. (They are just being polite and calling it "unnecessary" or "superfluous" instead of "pernicious".) Alas, programmers that keep making this mistake tend to have that warning disabled, too.