Question

I am having a discussion with my co-worker on how much work a constructor can do. I have a class, B that internally requires another object A. Object A is one of a few members that class B needs to do its job. All of its public methods depends on the internal object A. Information about object A is stored in the DB so I try to validate and get it by looking it up on the DB in the constructor. My co-worker pointed out that the constructor should not do much work other than capture the constructor parameters. Since all of public methods would fail anyway if object A is not found using the inputs to the constructor, I argued that instead of allowing an instance to be created and later fail, it is actually better to throw early in constructor. Not all classes do this but i found that StreamWriter constructor also throws if it has problem opening a file and doesn't delay the validation until the first call to Write.

What others think? I am using C# if that makes any difference.

Reading Is there ever a reason to do all an object's work in a constructor? i wonder fetching object A by going to DB is part of "any other initialization necessary to make the object ready to use" because if user passed in wrong values to the constructor, i wouldn't be able to use any of its public methods.

Constructors should instantiate the fields of an object and do any other initialization necessary to make the object ready to use. This is generally means constructors are small, but there are scenarios where this would be a substantial amount of work.

Was it helpful?

Solution

Your question is composed from two completely separate parts:

Should I throw exception from constructor, or should I have method fail?

This is clearly application of Fail-fast principle. Making constructor fail is much easier to debug compared to having to find why the method is failing. For example, you might get the instance already created from some other part of code and you get errors when calling methods. Is it obvious the object is created wrong? No.

As for the "wrap the call in try/catch" problem. Exceptions are meant to be exceptional. If you know some code will throw an exception, you do not wrap the code in try/catch, but you validate parameters of that code before you execute the code that can throw an exception. Exceptions are only meant as way to ensure the system doesn't get into invalid state. If you know some input parameters can lead to invalid state, you make sure those parameters never happen. This way, you only have to do try/catch in places, where you can logically handle the exception, which is usually on system's boundary.

Can I access "other parts of the system, like DB" from constructor.

I think this goes against principle of least astonishment. Not many people would expect constructor to access a DB. So no, you should not do that.

OTHER TIPS

Ugh.

A constructor should do as little as possible - the issue being that exception behavior in constructors is awkward. Very few programmers know what the proper behavior is (including inheritence scenarios), and forcing your users to try/catch every single instantiation is... painful at best.

There's two parts to this, in the constructor and using the constructor. It's awkward in the constructor because you can't do much there when an exception occurs. You can't return a different type. You can't return null. You basically can rethrow the exception, return a broken object (bad constuctor), or (best case) replace some internal part with a sane default. Using the constructor is awkward because then your instantiations (and the instantiations of all derived types!) can throw. Then you can't use them in member initializers. You end up using exceptions for logic to see "did my creation succeed?", beyond the readability (and fragility) caused by try/catch everywhere.

If your constructor is doing non-trivial things, or things that can reasonably be expected to fail, consider a static Create method (with a non-public constructor) instead. It's much more usable, easier to debug, and still gets you a fully constructed instance when successful.

Constructor - method complexity balance is indeed a matter of discussion about good stile. Quite often empty constructor Class() {} is used, with a method Init(..) {..} for more complicated things. Among arguments to be taken into account:

  • Class serialization possibility
  • With the separation Init method from constructor, you often need to repeat same sequence Class x = new Class(); x.Init(..); many times, partially in Unit Tests. Not so good, however, crystal clear for reading. Sometimes, I just put them in one line of code.
  • When Unit Test aim is just Class initialization test, better to have own Init, to do more complicated actions fulfilled one-by-one, with the intermediate Asserts.
Licensed under: CC-BY-SA with attribution
scroll top