23

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.

Improve this question
9
  • 8
    I disagree. Have a look at dependency inversion principle, it would be better if object A is handed to object B in a valid state in it's constructor.
    – Jimmy Hoffa
    Commented Jan 13, 2014 at 18:50
  • 5
    Are you asking how much can be done? How much should be done? Or how much of your specific situation should be done? That's three very different questions.
    – Bobson
    Commented Jan 13, 2014 at 18:52
  • 1
    I agree with Jimmy Hoffa's suggestion to look at dependency injection (or "dependency inversion"). That was my first thought on reading the description, because it sounds like you're already half way there; you already have functionality separated out into class A, which class B is using. I can't speak to your case, but I generally find that refactoring code to use the dependency injection pattern makes classes simpler / less intertwined.
    – Dr. Wily's Apprentice
    Commented Jan 13, 2014 at 19:43
  • 1
    Bobson, i changed the title to 'should'. I am asking for best practice here.
    – Taein Kim
    Commented Jan 13, 2014 at 20:50
  • 1
    @JimmyHoffa: maybe you should expand your comment into an answer.
    – kevin cline
    Commented Jan 13, 2014 at 21:53

3 Answers 3

Reset to default
24

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.

Improve this answer
11
  • 2
    The more appropriate solution to this is usually to add an "open" type method where construction is free, but no use can be done before you "open"/"connect"/etc which is where all the actual initialization occurs. Constructors failing can cause all sorts of issues when you in the future want to do partial-construction of objects which is a useful ability.
    – Jimmy Hoffa
    Commented Jan 13, 2014 at 18:53
  • @JimmyHoffa I see no difference between constructor method and specific method for construction.
    – Euphoric
    Commented Jan 13, 2014 at 18:57
  • 5
    You may not, but many do. Think about when you create a connection object, does the constructor connect it? Is the object usable if it's not connected? In both questions the answer is no, because it's helpful to have connection objects partially-constructed so you can tweak settings and things before you open the connection. This is a common approach to this scenario. I still think constructor injection is the right approach for scenarios where object A has a resource dependency but an open method is still better than having the constructor do the dangerous work.
    – Jimmy Hoffa
    Commented Jan 13, 2014 at 18:59
  • @JimmyHoffa But that is specific requirement of connection class, not general rule of OOP design. I would actually call it an exceptional case, than a rule. You are basically talking about builder pattern.
    – Euphoric
    Commented Jan 13, 2014 at 19:11
  • 2
    It's not a requirement, it was a design decision. They could have made the constructor take a connection string and connect immediately when constructed. They decided not too because it's helpful to be able to create the object, then tweak figure out the connection string or other bits and bobbles and connect it later,
    – Jimmy Hoffa
    Commented Jan 13, 2014 at 19:14
20

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.

Improve this answer
9
  • 2
    Complex construction scenarios are exactly why creational patterns like you mention here exist. This is a good approach to these problematic constructions. I'm given to think about how in .NET a Connection object can CreateCommand for you so that you know the command is appropriately connected.
    – Jimmy Hoffa
    Commented Jan 13, 2014 at 18:57
  • 2
    To this I would add that a Database is a heavy external dependance, suppose you want to switch databases at some point in the future (or mock an object for testing), moving this kind of code to a Factory class (or something like an IService provider) seems like a cleaner approach
    – Jason Sperske
    Commented Jan 13, 2014 at 18:58
  • 4
    can you elaborate on "exception behavior in constructors is awkward"? If you were to use Create, wouldn't you need to wrap it in try catch just like you would wrap call to constructor? I feel like Create is just different name for constructor. Maybe i am not getting your answer right?
    – Taein Kim
    Commented Jan 13, 2014 at 19:01
  • 1
    Having had to try catching exceptions bubbling up from constructors, +1 to the arguments against this. Having worked with folks that say "No work in the constructor" though, this can also create some weird patterns. I have seen code where every object has not only a constructor, but also some form of an Initialize() where, guess what? The object isn't really valid until that's called either. And then you have two things to call: the ctor and Initialize(). The problem of doing work to set up an object doesn't disappear - C# may yield different solutions than say C++. Factories are good!
    – J Trana
    Commented Jan 14, 2014 at 5:51
  • 1
    @jtrana - yeah, initialize is no good. The constructor initializes to some sane default or a factory or create method builds it and returns a usable instance.
    – Telastyn
    Commented Jan 14, 2014 at 12:24
0

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.
Improve this answer
4
  • the advantage, in my opinion, of the init method is that handling failure is a lot easier. In particular, the return value of the init method can indicate whether initialization is successful, and the method can ensure the object is always in a good state.
    – pqnet
    Commented Mar 12, 2018 at 11:27
  • @pqnet With init() methods I see the issue that you end up with temporal coupling. You can do X with this class, but you have to init() it first. So now you need to add checks to every function call if the class is really initialized and handle the possible failure events...
    – CharonX
    Commented Jan 28, 2020 at 15:28
  • @pqnet: if init() fails, what then? Throw an exception? What's the advantage exactly? You just lost the stack trace along with everything else, and why couldn't that have been handled just as well in the constructor to begin with? Useless wrapper methods are a pain
    – Ed Swangren
    Commented Oct 11, 2022 at 0:15
  • @pqnet: "and the method can ensure the object is always in a good state" - like a constructor is meant to do?
    – Ed Swangren
    Commented Oct 11, 2022 at 1:13

Not the answer you're looking for? Browse other questions tagged or ask your own question.