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

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

Local variables should always be initialized, right?

Again and again I see programmers doing the following:

{ 
    string emailAddress = string.Empty; 

    if( /*condition*/ ) 
    { 
        //do some stuff 
        emailAddress = retrieve_email_address(); 
        //use emailAddress 
    } 
}

You may see it with setting strings to String.Empty as in the example, or you may see it with setting pointers to null, integers to zero, etc. There are two problems here; the first one is easy to see, the second one might not be so easy.

First of all, emailAddress was declared outside of the scope in which it is being used. That's not a very good idea; it should have been declared within the scope in which it is being used, so the code should instead read string emailAddress = retrieve_email_address(); That one was simple. But if we ignore it for a moment, there is another, actually worse problem with the above code.

Some people believe that when declaring a local variable you should always initialize it with some initial value, and they follow this rule with superstitious devotion. This is dead wrong. It may have been advisable back in the dark ages of the first C compilers, but not anymore.

Modern compilers of C, C++, C# and Java are quite good at warning you if a variable might be read before it has been initialized. So, by pre-initializing it at declaration time with a value which is by definition meaningless, (since a meaningful value is not yet known at that time,) 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 before you make use of it, the compiler will not warn you, because as far as the compiler knows, the variable has already been initialized at declaration time.

(That was from an answer of mine on CodeReview.StackExchange.com.)

Furthermore, when a programmer sees a value being initialized to a certain value, they are inevitably tempted to think that this value has a certain role to play in the algorithm which follows. For example, setting an integer to zero makes one think that what follows must be a loop using that integer as an accumulating sum. So, when that's what you expect, and you read further down only to find that the variable is re-initialized with a different value, it is rather disappointing.

-

2011-12-31

C# Blooper №1: No warnings about uninitialized readonly members when the class is public and the member is public, protected or protected internal.


Before reading any further, please read the disclaimer.

The C# compiler is kind enough to give you a "field is never assigned to" warning if you forget to initialize a readonly member which is private or internal, or if the class in which it is being declared is internal. But if the class is public, and the readonly member is public, protected or protected internal, then no warning for you! Why, oh why?

namespace Test1  
{  
    class Test1 
    {  
#if TRY_IT  
        public readonly int m; //OK: warning CS0649: Field is never assigned to, and will always have its default value 0  
        protected readonly int n; //OK: warning CS0649: Field is never assigned to, and will always have its default value 0  
        internal readonly int o; //OK: warning CS0649: Field is never assigned to, and will always have its default value 0  
        private readonly int p; //OK: warning CS0649: Field is never assigned to, and will always have its default value 0  
        protected internal readonly int q; //OK: warning CS0649: Field is never assigned to, and will always have its default value 0  
         
        Test1() 
        { 
            if( p != 0 ) //To avoid warning 'The field is never used' 
                return; 
        } 
#endif 
    }  
  
    public class Test2 
    {  
#if TRY_IT  
        private readonly int m; //OK: warning CS0649: Field is never assigned to, and will always have its default value 0  
        internal readonly int n; //OK: warning CS0649: Field is never assigned to, and will always have its default value 0  
 
        Test2() 
        { 
            if( m != 0 ) //To avoid warning 'The field is never used' 
                return; 
        } 
#endif  
        public readonly int o; //Blooper: no warning about field never assigned to.  
        protected readonly int p; //Blooper: no warning about field never assigned to.  
        protected internal readonly int q; //Blooper: no warning about field never assigned to. 
    }  
  
    public sealed class Test3 
    {  
        public readonly int m; //Blooper: no warning about field never assigned to.  
    }  
}  

For a moment you might think "well, a descendant might initialize that member", but that theory does not hold any water, for a number of reasons:
  • Internal classes may also be subclassed, but the compiler does not fail to issue the warning in their case.
  • Sealed classes may not be subclassed, but the compiler fails to issue the warning in their case, as Test3 in the sample code demonstrates.
  • The warning makes sense for the sake of the integrity of the base class regardless of what a derived class may or may not do.
  • Lastly but most importantly, the C# specification expressly prohibits a derived class from initializing a readonly member of a base class. You get Error CS0191: A readonly field cannot be assigned to (except in a constructor or a variable initializer) which, incidentally, is a little bit misleading, because you may be trying to assign the field from within a constructor, only it is the constructor of the wrong class.
According to MSDN Documentation about this warning, the exhibited behavior is to be expected:
Compiler Warning (level 4) CS0649:
Field 'field' is never assigned to, and will always have its default value 'value'
The compiler detected an uninitialized private or internal field declaration that is never assigned a value.
The question is: why?

UPDATE:

I posted this question on StackOverflow, and Eric Lippert himself answered it. The short answer is that it is an oversight of the compiler, but the long answer is also quite interesting and worth reading.
-

C# Bloopers


Please do not get me wrong; C# is awesome. It is the language of my choice, even though I am pretty well versed in C++ and Java. That having been said, it cannot be denied that C# has its share of flaws, too. In this series of posts I am documenting some of them, in no particular order.

C# Blooper №1: No warnings about uninitialized readonly members when the class is public and the member is public, protected or protected internal.

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

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

Stay tuned, there is more to come.
-

2011-09-15

Gerasko Aei Didaskomenos

Solon's original phrase was "Gerasko D' Aei Polla Didaskomenos", but here in Greece we use the shorter form, "Gerasko Aei Didaskomenos". I grow older forever learning new things.

It is never too late to learn something new. For example, just a few months ago I learned of the existence of the "??" operator in C#, after several years of using the language. Of course I had come across that operator in the reference, but it had not occurred to me how useful it would be in real-life scenarios. Then, one day I chanced upon someone else's code making use of it. There was a big 'aha!'.

So, today I learned one of the most useful things I know about Visual Studio.

Let us suppose for a moment that you are a real programmer, not a button-clicker that Microsoft thinks you are. So, when you double-click on a file in the Solution Explorer, what you want to see is the code, naturally. And this is what usually happens. But then one day you start programming Windows Forms, and suddenly double-clicking on some (but not all) of the files in your project does not show the code. Instead, Microsoft believes that what you must want to see in these cases, is, undoubtedly, their spiffy Windows Forms Designer. Because obviously, they think you must be a mindless button-clicker.

So, every time you double-click on a .cs file that happens to contain a form class, you are presented with a great big huge gotcha! as the designer appears before you instead of the code editor, and you are reminded, for the thousandth time, that double-clicking on a .cs file will do vastly different things, depending on what the class inside that .cs file derives from.

So, is there a way to convince Visual Studio to always show you the code instead of the designer when you double click on an item in solution explorer? In other words, is there any way to tell Visual Studio that you are a real programmer and not the button-clicker that Microsoft thinks you are?

It turns out that there is. I found it out today thanks to StackOverflow, (see here and here,) and it really made my day. In short, here it is:

The next time you want to open a .cs file, right-click on it and select "Open With..."; on the dialog that opens, select "CSharp Editor", and then click on the button which says "Set as Default". That's it, it will work forever after for all .cs files.

The only problem now is, that after a couple of years of Windows Forms Programming while being conditioned to always be careful to right-click on forms .cs files and select 'View Code', I have to first unlearn that before I can start double-clicking freely again.

Cheers!