Welcome to Pete Brown's 10rem.net

First time here? If you are a developer or are interested in Microsoft tools and technology, please consider subscribing to the latest posts.

You may also be interested in my blog archives, the articles section, or some of my lab projects such as the C64 emulator written in Silverlight.

(hide this)

Choices I hate to make: Constructor exceptions vs. Initialize patterns

Pete Brown - 03 October 2011

I'm writing a little sample for Silverlight 5 in Action. Part of it involves a class which integrates with a 3d accelerometer sensor on Windows. In order to use the accelerometer, I have to check to see if elevated permissions is enabled before I go about constructing objects which rely on COM automation. There's also a fair bit of construction activity after that check to create the actual sensor object and prepare the class for use.

There are three approaches to creating this object and performing the initialization. All three have issues. Here they are.

Code in the Constructor

It "feels" right to handle the permissions check in the constructor. However, throwing exceptions from a constructor is generally considered a bad… no, an EVIL practice. The code in this example is the smallest, and tends to be the most easily understood by other developers. Here's an example of what the code might look like (don't get hung up on the specifics of the code itself, as it happens to use the Native Extensions for Silverlight, but could be any similar functionality) :

public class AccelerometerJoystick
{
private SensorManager _sensorManager = null;
private Sensor _accelerometer = null;
private PropertyKey _keyXGs = null;
private PropertyKey _keyYGs = null;

public AccelerometerJoystick()
{
if (Application.Current.HasElevatedPermissions)
{
_sensorManager = new SensorManager();

// do a bunch of other stuff to create a Sensor instance.
}
else
{
// throw an exception that says you need permissions
}
}


public void DoSomething()
{
// ...
}
}

The constructor exception issue is especially nasty if the object is constructed in XAML. That's not my intent with this class, but you never can know exactly how your class will be used.

It's also nasty because your construction code needs to be in a try/catch, which isn't always intuitive. Rightly or wrongly, many folks put construction outside the try/catch block, and expect only actual methods to throw exceptions.

Using an Initialize Pattern

Another approach is to use an initialize pattern. I've hated this pattern since the day we started using it Visual Basic (4, I think) when we had classes without real constructors. Basically, this requires a two-step process. First, the caller creates an instance of the object, then the caller calls the Initialize method which performs the initialization and sets a flag. Because the caller could forget to call Initialize, the code inside the class needs to do a check against that flag (or an object being not null) in every method.

public class AccelerometerJoystick
{
private SensorManager _sensorManager = null;
private Sensor _accelerometer = null;
private PropertyKey _keyXGs = null;
private PropertyKey _keyYGs = null;
private bool _initialized = false;

public AccelerometerJoystick()
{
// pretty empty
}

public void Initialize()
{
if (Application.Current.HasElevatedPermissions)
{
_sensorManager = new SensorManager();

// do a bunch of other stuff to create a Sensor instance.

_initialized = true;
}
else
{
// throw an exception that says you need permissions
}
}

public void DoSomething()
{
if (_initialized)
{
// ...
}
else
{
// throw exception saying you're not initialized
}
}
}

For the record, I absolutely LOATHE this pattern. I hate that the caller has to have a regular construction statement followed by an Initialize statement.

You can also use a lazy initialize pattern where instead of the caller calling Initialize, you check in each method and perform the initialization if not already initialized. I'm not a big fan of this approach either.

Using a Simple Factory Pattern

Yet another option is to use a factory pattern. I think factory patterns tend to be overused. However, like the Initialize pattern, this is something we've used since the old VB days when we didn't actually have constructors. It's ok in that you don't have to remember to call an initialize method, and don't need all those checks in your code, but you can still get an exception on the actual construction.

With this approach, you need a private constructor and a public static method to handle the construction.

public class AccelerometerJoystick
{
private SensorManager _sensorManager = null;
private Sensor _accelerometer = null;
private PropertyKey _keyXGs = null;
private PropertyKey _keyYGs = null;

private AccelerometerJoystick()
{
// pretty empty, but private
}

public static AccelerometerJoystick Create()
{
if (Application.Current.HasElevatedPermissions)
{
var joy = new AccelerometerJoystick();

joy._sensorManager = new SensorManager();

// do a bunch of other stuff to create a Sensor instance.

return joy;
}
else
{
// throw an exception that says you need permissions
}
}

public void DoSomething()
{
}
}

This is more convoluted, especially when compared to the constructor approach. It also has a habit of becoming the big sledgehammer you use throughout your code, and I can't stand that. IMHO, this approach is probably the safest, but it still doesn't smell quite right to me.

What would you do, and why?

         
posted by Pete Brown on Monday, October 3, 2011
filed under:          

21 comments for “Choices I hate to make: Constructor exceptions vs. Initialize patterns”

  1. Jeremy Liknesssays:
    Most of the time when I have classes like that, I'm getting them through the IoC container. In the case of MEF, for example, I can use an export factory to create instances and implement IPartImportsSatisfiedNotification that is called not when it is constructed, but after dependencies have been resolved and it's available for use. This is the logical place to put "initialization code" and do it in a way that you don't have to worry about the caller knowing to call initialize ... in essence, the caller just imports the reference, and the framework handles calling the import notification, so everyone is happy - it's like the factory approach without you having to write the factory.
  2. Philsays:
    Hey Pete,
    Personally I don't have any issue with throwing an exception from a constructor (in C#). I'm pretty sure there are lots of BCL constructors that throw exceptions if input parameters are invalid. The FileStream springs to mind as a good example where this happens.

    Not sure if this is a thoroughly convincing argument, but in my opinion it's by far the best way forward out of the three patterns above; followed closely by the factory pattern.

    Just my 2 Pence worth. :)
  3. Rob Sedersays:
    Pete,

    First, just to split hairs - it IS appropriate to throw things like ArgumentException if a constructor has invalid arguments, so it's not always a bad idea. In this case though, I do agree.

    I have a different perspective of your problem. You are trying to initialize "stuff" when an instance of the class is created, which is not when you should "validate" that it's ready to run, nor when you need it initialized - so I would argue that none of this code should be in a constructor or initialize or anything like that.

    I would suggest you create a private void Initialize() method that checks to see if things have already been initialized, if so, then return. If not, then initialize.

    Now, from the method in this class that actually DOES something - have THAT method call this Initialize() method blindly (and of course bubble-up proper/wrapped exceptions. Even if this class will be used to only process events, you would still likely have a .Start() and .Stop() - all this initialize business should be there, when you intend to use it - NOT in or related to the construction of the object.

    That's my $.02, I hope that helps

    -Rob
  4. Mark Lindellsays:
    First, I would never use public Initialize methods. Can you say candy machine interfaces?

    In this case simple case I would throw an exception from the constructor. The BCL does this in Guid, FileStream and String to name a few.

    Ask the question: Is it valid to have an instance of an AccelerometerJoystick object that can not be used? Just deferring the call so the exception does not happen in the constructor does not seem like a good approach. Why start with an object instance that cannot be used unless you expect the value of Application.Current.HasElevatedPermissions to change after construction. Then the deferred initialization mentioned by Rob would be appropriate.

    As the work grows I would consider using a factory method approach.

    I agree with Jeremy if you are using the Ioc approach.

    Oh, and I would never take a dependency on a static. Pure Evil!



  5. Davesays:
    In this case the Factory Pattern is the best option. Think (Web)HttpRequest.Create which does pretty much the same thing. Factory Patterns are not a bad thing by default, but like all design patterns you really have to work out if the pattern you're using is really the best one.

    Exceptions from a constructor are a bad practive and a Initialize() method is a dependency call which should be avoided. You could refactor Initialize to an EnsureInitialized() method which is called on every property or method of the instance class, but that create a lot of overhead because of multiple IsInitialized (private property) calls..

    So by using the process of elimination, the Factory Pattern is the best option..
  6. Alessandrosays:
    Your code example isn't one of the best. From the looks of it, your joystick class is only useful when your application has elevated permissions.

    The consumer of your class needs to be informed of this. The responsibility to check for elevated permissions is supposed to be done by the consumer and not by the joystick class itself. Throw an exception in the DoSomething() method, in case elevated permissions are not met and the problem magically goes away.
  7. Willsays:
    The xaml serializer respects ISupportInitialize, so if you are instantiating your types in xaml ISI is definitely the way to go (nothing worse than a constructor exception in a type instantiated in xaml!).
  8. wekempfsays:
    Why do you (and others) presume that throwing exceptions from a constructor is bad? Like others have pointed out, several BCL classes do this. Further, the Framework Design Guidelines states emphatically "DO throw exceptions from instance constructors, if appropriate." I can't fathom any reasoning that would suggest throwing from constructors would be problematic in any way.
  9. wekempfsays:
    Having read everything thoroughly now, I don't see an issue. Throw from the constructor. This is part of the contract for using the class (@Consulting Machanic hints at this when suggesting using Code Contracts). Failure of the consumer to meet this contract is a bug and won't happen with properly coded consumers (@Alessandro suggests this, though I disagree with his recommendation to throw from DoSomething... the contract was violated much earlier than that). Even if you could convince me that there's problems with throwing exceptions from constructors (don't buy that at all), this would be an exception to your reasoning. Contract violations are software bugs. You don't catch such exceptions (you should never catch an ArgumentException!) and try to recover from them, you let the exception propogate all the way to application termination (speaking in general terms... critical software obviously needs some safe mechanism for recovery, but the point is, this is a BUG and not some I/O error or environmental failure).
  10. Petesays:
    @Rob

    That sounds like the lazy initialize approach I mentioned in the post. Not my favorite by far.

    @Alessandro

    It's a real example; they're never the best :)

    I disagree that the exception should be thrown in DoSomething in this case. I can't even create the COM server without elevated permissions.

    @Will

    Excellent point on ISupportInitialize

    @Wekempf

    Many people consider constructor exceptions (except things like argument exception) a bad practice, which is why I threw this out there. I mentioned some reasons in the text. Thanks for your great explanation, though :)

    FWIW, for the example, I ended up just putting the code in the constructor.
  11. wekempfsays:
    I'm interested in WHY they think it's a bad practice, though. I'll admit, this comment surprised me. I immediately started doing some web searches, and all I could conclude is that you're correct, several people think it's a bad idea. However, I'm finding very little reasonging behind this opinion. It seems to be a cult opinion, rather than a reasoned opinion. If there are reasons, I'd like to know what they are, to further my own education (even if I conclude those reasons are faulty, you can learn from them).

    BTW, like I said in my response before, I think this is exactly like an ArgumentException. In DbC, pre-conditions often depend on more than the parameters passed. In your case, the API requires the environment to be running elevated, and that's a pre-condition the same as needing the foo parameter to be non-null. So, obviously, I think you might the right decision. :)
  12. Maxwell Cabralsays:
    Not really sure why throwing from the constructor in C# or C++ is bad. It seems to even be promoted in a few texts/internet C++ FAQs. Maybe this is confusion with throwing an exception from the C++ destructor which isn't normally applicable with C# (unless your own Dispose method is throwing an exception, i.e. http://stackoverflow.com/questions/1030455/how-to-handle-exception-thrown-from-dispose).

    The reason I personally tend to avoid throwing exceptions from constructors unless its the best thing to do is that someone ends up writing something like this:

    try{
    MyObject EpicFail = new MyObject(JunkData);
    } catch {
    //To do: add exception handling logic.
    }

    This stuff shouldn't happen of course and is an institutional issue.

    I tend to avoid needing a lot of exception throwing/handling by/in constructors by validating my data before any objects consume it or defer a lot of that logic to object serializers/deserializers and catch exceptions generated by them/fix the data (At work I usually have full control over the data or can yell at someone down the hall - lucky I know). In addition consumers of my work don't have to write their own validation logic each time they want to use my library.

    For cases where it's not data or something critical to the software (say visual garnish), I'll turn to lazy initialization, erring on the side of creating a mostly useless zombie object and either trying to load it again later (say if someone changes a theme and the garnish changes). I don't see any good reason to spend cycles try-catching the construction of something that will work 99% of the time and failure just makes it less beauti-magical. Worse yet is that if I throwing an exception that one of my co-workers won't bother to catch, it might take down the program while its doing something important. The image class in Silverlight doesn't take down your app because you threw a bad url at it.

    Finally if it's something critical, I will throw an exception and warn people in documentation that the gods will smite them if they don't handle it.

    The (practical) joys of working with others :).
  13. Marc Drossaerssays:
    Intriguing problem!

    For problems like this, I often take a look at “The C++ Programming Language” by Bjarne Stroustrup. With respect to this problem one finds that:

    - Exceptions exist to communicate problems / errors, up the calling tree to where appropriate measures can be taken. It is the best known mechanism.
    - An exception should be thrown when a constructor cannot realize its class invariant, or cannot construct the class – e.g. because it cannot obtain a resource.
    - Throwing an exception from a constructor can lead to failure to release (unmanaged) resources (in badly written code). My guess is that’s why it is undesirable to throw exceptions from constructors.

    I’ve tried to write a simple Silverlight application, that instantiates an object which throws an exception from the constructor, that requires elevated permissions, can be tested in deployed state, and creates a mess. I had to give up due to time limits :-).
  14. Jonathan van de Veensays:
    The reason that it is unwise to throw an exception in a constructor in C++ is that if construction of the object failes (an exception did occur), the deconstructor is never called. If something was 'newed-up' in the constructor, that object will still be around and can't be destroyed in any way (as there is no pointer) and now you have a memory leak.

    Obviously with a garbage collector (like in C#) you don't have this problem as it will simply remove any unused references automatically.

    I do think that if you feel the need to throw an exception from a constructor you should do so at the very start of your constructor to prevent doing work you won't be using. Also, obviously all the other rules for throwing exceptions should still apply (something is going on that should NEVER occur, you can not resolve the issue in any other way, you're at the right level of abstraction, include all the information you have that led to the exception).
  15. Rob Relyeasays:
    Hey Pete-
    Interesting to read opinions here. I was confronted with this case yesterday when I modifying the ShapeGame sample in the Kinect SDK (for the next beta).

    I ended up creating a public static factory method Create(), making the ctor private.
    In this case, it is not extremely unusual for creation to fail, because one of the Speech prerequisites may be missing.
    I chose to not require the mainWindow.xaml.cs coder to have to try/catch...but instead to check for null from the Create method.

    Would love opinions on this kind of case...(using null).
    Thx, Rob
  16. Mylessays:
    I would probably be using an IOC container, so none of the above really appeal as solutions. In this case I could then return 2 different implementations depending on the security level.

    The other alternative too this would be to use the null pattern to return a null version version of the SensorManager (or perhaps the containing class) - this is however really just another variation to the above.

    Hate the idea fo the Initialze() method - that is really exposing the implemenation to other classes.
    The question that has to be asked is what should be the business/application logic if the user does not have the required persmissions - this would probably guide me in choosing between the other options.

    Myles.
  17. Richardsays:
    How about a TryCreate factory method that returns null if you don't have elevated permissions?

    var joy = AccelerometerJoystick.TryCreate();
    if (null != joy)
    {
    // Do stuff...
    }

    That's got to be better than:

    try
    {
    var joy = AccelerometerJoystick.Create();
    // or: var joy = new AccelerometerJoystick();
    // Do stuff...
    }
    catch (SomeException)
    {
    // Don't do stuff...
    }

    or even:

    var joy = new AccelerometerJoystick();
    try
    {
    joy.DoSomething();
    }
    catch (SomeException)
    {
    }
  18. Ranjansays:
    Although the third approach is way better than others however, the code smells when there's a direct dependency on the permission check using - Application.Current.HasElevatedPermissions

    I would ask myself How would I unit test this code if at all I wish to do one ?

    Even though the AccelerometerJoystick has to check whether it has right permission before doing something, but it does not make sense to have a direct permission check on Application.Current instance.

    I hope you get what I'm talking about :)

Comment on this Post

Remember me