Order Matters

Today I found what was probably not the best of coding but for sure should have been flagged by the compiler and not the exception handler - especially the type of exception. Here's my class:



public
class foo
{
   // ... other definitions here
   public static readonly Font CellStyle_UnacknowledgedAlarm_Font = new Font(CellStyle_Font, FontStyle.Bold);
   public static readonly Font CellStyle_Font = new Font("Verdana", 12);

   private foo() { /*Cannot instantiate*/ }
}

 



You'll noticed the top to bottom issue that CellStyle_Font is referenced before defined.

This compiles wonderfully, but when I use any of the definitions in this class (not necessarily these) I get an exception generated at run time: TypeInitializationException was unhandled. The inner exception for this is "Object reference not set to an instance of an object."

All that said, if I reorder the two lines the exception goes away. This took quite a while to track down as the exception really gave me no related information to the problem.

Any ideas




Answer this question

Order Matters

  • faiz ahmed

    "10.4.5.1 Static field initialization" of the C# language spec. Check http://msdn.microsoft.com/vcsharp/programming/language/ for downloads.


  • Eric Latham

    An experiment using simple types other than Font work fine, so this looks like a problem with the Font class itself.

  • sniggihts

    I can verify that the definition of CellStyle_Font must precede that of CellStyle_UnacknowledgedAlarm_Font.

    Stack trace yields "   at System.Drawing.Font..ctor(Font prototype, FontStyle newStyle)\r\n   at TryStuff.foo..cctor()" and the InnerException data is {System.Collections.ListDictionaryInternal}

    Looks like you found one.


  • peancor

    As Peter said it is in the C# spec.  That's where I look things up when I want to know the details about the language.  Unfortunately, like most specs, it is a little hard to understand unless you are familiar with compiler technojumble :} 

    Michael Taylor - 10/30/05

  • Andrew Thorpe

    I disagree with Blair's comments about the threading.  C# guarantees that static fields are initialized prior to first use.  Furthermore statics (using the initializer syntax only) are thread-safe.  This is in fact how singletons are implemented prior to 2.0.  Therefore your initialization is fine.  Given that this is a readonly static field I also don't have a problem with exposing the field as public.  This is, in fact, recommended over exposing public constants.  A property serves no purpose here since you are exposing a static read-only field that can only be read.  Furthermore since Font is immutable exposing it as a read-only value truly makes sense. 

    However as Blair mentioned in general you shouldn't expose fields as public.  My rule of thumb is that I never expose fields either public or protected except if:
    a) It is read-only,
    b) It is constant, or
    c) It is a private struct or class that nobody but myself can use.  Even in this case I still use properties most of the time.

    Furthermore if you expose a reference type as a read-only field then be aware that the field is read-only but that doesn't stop me from setting the properties of the field to something.  It works for Font because Font is immutable.

    Back to your original question my initial hunch was that it was a compiler bug since C# guarantees that a static is initialized prior to first use.  However I use to write compilers for a living so I know that it is rarely that simple.  So I looked it up.  Static variables, like reference types, are automatically initially assigned and therefore contain a value.  In the case of your field it'll be null.  This is valid and meets the spec.  In fact the spec specifically says that the time of static initialization is undefined but will happen prior to first use of the field.  As a result the order in which statics are declared is irrelevant to the order in which they are initialized.  You lucked up in this case because the compiler probably does them in order but it is an implementation detail and therefore undefined.   Assuming that because you referenced  the static in  another static  and therefore it should work is really relying on how the spec reads and is generally not a good idea.

    The only way to guarantee the time in which statics are initialized is to implement a static constructor.  In this case all static fields are guaranteed to be initialized prior to invoking the static constructor.  This wouldn't help in your case since you have a static dependent upon a static.  The only workaround is that you should remove the initialization expression and use a static constructor to initialize the fields instead.  This makes your class standards-compliance, remains thread-safe and behaves as you expect.

    As a side note this is typical for a language (even C++).  Compilers must be able to reorder things like declarations and even statements in order to optimize performance and meet the requirements of the target machine.  It is one of the more common optimizations that compilers do.

    Michael Taylor - 10/28/05





  • Dips123

    Don't know if it is really a bug. . .

    Your declaration has some basic standard practice as well as thread safety issues. If you apply proper practice, the issue(s) goes away, I am sure.

    First (Standard Practice): Member variables should never be declared public, but should be exposed via properties.

    Second (Thread Issues) : Access to static values should be blocked first to assure that the value is actually instanced. Once instancing is assured, unblock and return the value.

    This is how I would make the above declaration:


    public class foo
    {
     private static readonly Font _cellStyle_UnacknowledgedAlarm_Font;
     private static readonly Font _cellStyle_Font;
     private class _syncCellFont{};
     private class _syncAlarmCellFont{};
     private foo(){} 
     static public Font CellStyle_Font
     {
      get
      {
       lock (typeof(_syncCellFont))
       {
        if(_cellStyle_Font == null) _cellStyle_Font = new Font("Verdana", 12);
       }
       return _cellStyle_Font;
      }
     }

     static public Font CellStyle_UnacknowledgedAlarm_Font
     {
      get
      {
       lock (typeof(_syncAlarmCellFont))
       {
        if(_cellStyle_UnacknowledgedAlarm_Font == null)
          _cellStyle_UnacknowledgedAlarm_Font = new Font(CellStyle_Font, FontStyle.Bold);
       }
       return _cellStyle_UnacknowledgedAlarm_Font;
      }
     }
    }

     



    Side issue, Do you really want a 'Global' instance of the font where the reference is common among all usage I could be wrong, but if you decided to make the color of one usage 'Red' wouldn't it change all the references

    Aren't you looking to 'Factory' fonts
    Something like this:


    public class foo
    {
     private foo(){} 
     static public Font CellStyle_Font
     {
      get
      {
       return new Font("Verdana", 12);
      }
     }

     static public Font CellStyle_UnacknowledgedAlarm_Font
     {
      get
      {
       return new Font(CellStyle_Font, FontStyle.Bold);
      }
     }
    }

     



  • DroopyMsdn

    I am wondering if it occurs with all marshalbyref derived classes

  • mqsash

    Just for fun, I ran FxCop over the code, which produced these Warnings (among others):< xml:namespace prefix = o ns = "urn:schemas-microsoft-com:office:office" />


    CA1052 : Microsoft.Design : Mark 'foo' as sealed.


    CA2104 : Microsoft.Security : Remove the readonly declaration from 'foo.CellStyle_Font' or change the field to one that is an immutable reference type. If the reference type 'System.Drawing.Font' is, in fact, immutable, exclude this message.


    CA1707 : Microsoft.Naming : Remove all underscores from member 'CellStyle_Font'.


    CA2104 : Microsoft.Security : Remove the readonly declaration from 'foo.CellStyle_UnacknowledgedAlarm_Font' or change the field to one that is an immutable reference type. If the reference type 'System.Drawing.Font' is, in fact, immutable, exclude this message.


    CA1707 : Microsoft.Naming : Remove all underscores from member 'CellStyle_UnacknowledgedAlarm_Font'.



  • the_grove_man

     TaylorMichaelL wrote:
    Furthermore since Font is immutable exposing it as a read-only value truly makes sense.


    I didn't even think about that!

    Where did you 'look' up this information about static initialization I couldn't find anything in the help files or any of my books. Though I did find reiterated your recommendation that initialization should occur in a static constructor, it was done with no explanation as to why. So thank you! 

    (I did find an attribute I didn't know existed - ThreadSafe)

    cheers!!!

  • Order Matters