7
\$\begingroup\$

I have a constructor which sets a member variable. The setting of this variable throws an exception. Should I be throwing the exception in the contructor or enclosing the try/catch like below. Is below snippet an example of code smell ?

private NewManager newManager;

    public Save(final TheManager manager, final String str) {
        this.manager = manager;

        try {
            this.newManager = manager.getManager(str);
        } catch (Exception e) {
            e.printStackTrace();
        } 
    }
\$\endgroup\$
  • 3
    \$\begingroup\$ I am confused about what exactly is a constructor in the supplied code, but my impression is that in this case manager.getManager(str) should be called before the constructor gets called - thus there will not be an exception happening inside of the constructor. Sometimes the constructor has to throw an exception, but as you might guess - it is better to avoid it if you can without too much effort. \$\endgroup\$ – Leonid May 29 '12 at 2:52
  • \$\begingroup\$ But doesn't placing a try/catch block in a constructor cause memory leaks? Please confirm guys and gals. \$\endgroup\$ – Geoffrey Jul 3 '15 at 9:54
  • \$\begingroup\$ This looks more like a generic best-practices question than a request to review your code, especially since you didn't include the implementation of TheManager.getManager(). Also, NewManager, TheManager, and Exception looks like placeholders. \$\endgroup\$ – 200_success Jul 3 '15 at 10:05
23
\$\begingroup\$

Catching an exception and just printing stack trace and proceeding like nothing happened is certainly Very Bad. What if no one is watching the standard error stream (where e.printStackTrace() dumps its output)? In your case, you'll probably end up with NullPointerException later on, since newManager is not set. It could be one second from now, or one year from now, depending on when someone is trying to use newManager. You want to fail fast instead.

There's nothing wrong in throwing exceptions from constructors. Just make sure that, wherever you catch it, you handle it appropriately.

| improve this answer | |
\$\endgroup\$
6
\$\begingroup\$

Catching an exception should be done for one of two reasons.

  1. You want to log it before rethowing.
  2. You know how to handle the exception.

If you don't know how to deal with an exception, catching it puts you into an unstable situation. You want to fail as quickly as possible for several reasons.

Let me be clear: this does not mean you have to "fix" the exception, it means you know what to do to allow the application to continue to operate appropriately. If you don't know the consequences of the application continuing after an exception, then do not catch it, you're just setting yourself up for further problems.

But if you can continue with reduced functionality or otherwise deal with the situation in an appropriate manner, then you should catch the exception and continue.

| improve this answer | |
\$\endgroup\$

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