8 thoughts on “Exception in the constructor – a NO-NO !”

1. If i understand correctly , when we use Using(CustomerWriter cw = new CustomWriter(@"c:\oren.txt",x)) and we some error to display and throw , we need to take care to close the connection somewhere , because the exception skip the Dispose Method, am i right?

2. The real problem is that you can’t really catch it anymore and call the Dispose() method as the instance (in your code – "cw") isn’t available outside of the "using" scope so it’s not possible to do something like:
   try
   {
   using (CustomWriter cw = new CustomWriter("…", 1))
   {
   //…
   }
   }
   finally
   {
   // cw isn’t available here.
   }

   The same applies to try-finally instead of the using, i.e:
   try
   {
   CustomWriter cw = null;
   try
   {
   cw = new CustomWriter("…", 1))
   //…
   }
   finally
   {
   if (cw!=null)
   cw.Dispose();
   }
   }
   finally
   {
   // cw isn’t available here.
   }

   In the latter example, you can put the line "CustomWriter cw = null;" before the first "try" and than catch it in the last finally, but this is simply worst practice scenario.

3. An elegant solution to this problem is the Constructor Method Pattern:

   public class CustomWriter : IDisposable
   {
   private StreamWriter Writer m_sw;

   CustomWriter()
   {
   }

   public static CustomWriter create(string filePath, int numberOfLinesToWrite)
   {
   CustomWriter obj = new CustomWriter();
   obj.m_sw = new StreamWriter(filePath);

   if (numberOfLinesToWrite < 0)
   {
   throw new ArgumentException("What’s wrong with you ?!!? numberOfLinesToWrite can’t be less than 0", "numberOfLinesToWrite");
   }

   // Assume that I’m using numberOfLinesToWrite here…
   return obj;
   }
   //…….

   Calling an initializer after constuction is a hassle…

   Don’t forget to make the actual constructor private.

4. Hey Yoni,
   thanks for the idea – nice touch !

5. Yoni – after thinking about it today in the train (on my way to work) it came up – your solution still won’t work in the given context (fix me if I’m wrong) as you would write it:
   using (CustomWriter cw = CustomWriter.Create("..", -5))
   {
   }

   This would still throw an exception and the Dispose method would not be called. The only way to make it work is via "try-finally".

   I must make myself clear though: The consumer of the class should not expect an error thrown from the constructor of the object! There is no way I’ll write only "try-finally" instead of "using" (depends on the case, of course) so I’ll make sure I’m clear – not if it’s depends on me anyway (meaning – this is one of the things in my code review items).

6. Yes I see what you mean. In that case let me first point out that it is a best practise to use a destructor to free unmanaged resources even when a Dispose method is implemented. The main reason is that you don’t want to force the client to be aware this object uses unmanaged resources and should be disposed of cleanly. (the less the user of an object has to know about it the better). This will solve the ‘using’ problem.

   The Constructor Method Pattern (in this context) was originally intended for languages that don’t call the destructor when an exception is thrown in the constructor (C++ for example). The CLR, however, does this and thus by implementing a destructor (which you should anyway) you can throw exceptions from constructors freely.

   I like the Constructor Method Patten mainly because I expect real constructors to be very slim and safe – the static method lets me know something a bit more heavy is probably performed except initialization.

   I have some difficulty with the statement: "The consumer of the class should not expect an error thrown from the constructor of the object!"
   Exceptions are generally unexpected and construction of a valid object can cause them no matter how hard you may try to aviod it. I think I’d rather have the exception than the uninitialized object, the latter being unsafe and having unexpected behavior… This may be a personal preference.

7. You should consider making internal pre-exception throwing handeling. (Fancy name for makign sure you close all resources before throwing an exception)

   public class CustomWriter : IDisposable
   {
   private StreamWriter m_sw;

   public CustomWriter(string filePath, int numberOfLinesToWrite)
   {
   m_sw = new StreamWriter(filePath);

   if (numberOfLinesToWrite < 0)
   {
   CloseResources();

   throw new ArgumentException("What’s wrong with you ?!!? numberOfLinesToWrite can’t be less than 0", "numberOfLinesToWrite");
   }

   // Assume that I’m using numberOfLinesToWrite here…
   }

   #region IDisposable Members

   public void Dispose()
   {
   CloseResources();
   }

   private void CloseResources();
   {
   // Time to say goodbye
   m_sw.Close();
   }

   #endregion
   }

8. Yoni – "The Constructor Method Pattern (in this context) was originally intended for languages that don’t call the destructor when an exception is thrown in the constructor (C++ for example). The CLR, however, does this and thus by implementing a destructor (which you should anyway) you can throw exceptions from constructors freely."
   I’ve tested it and throwing an exception from the constructor don’t auto’ calls my destructor. To the best of my knowledge, the destructor is nondeterministic so the GC will call it eventually but not immediately after an exception is thrown from the constructor. Am I missing something ?
   About the sentence: "The consumer of the class should not expect an error thrown from the constructor of the object!" – I can see your point and I agree with you. Still, the programmer should implement the constructor better and call the Dispose before throwing the exception – more on my post – "Best Practice: verify the safe cleanup of your unmanaged code".

   Justin – Thanks for bringing it up; See my post – "Best Practice: verify the safe cleanup of your unmanaged code".

Comments are closed.