213

Is it a good practice to make the constructor throw an exception? For example I have a class Person and I have age as its only attribute. Now I provide the class as

class Person{
  int age;
  Person(int age) throws Exception{
   if (age<0)
       throw new Exception("invalid age");
   this.age = age;
  }

  public void setAge(int age) throws Exception{
  if (age<0)
       throw new Exception("invalid age");
   this.age = age;
  }
}
7
  • 31
    Looks fine to me, but your duplication of code is bad practice. Just call setAge from your constructor to reduce a lot of duplicate code
    – Codemwnci
    Commented May 22, 2011 at 6:06
  • 26
    Might be a good idea to throw IllegalArgumentException in this case, makes it very explicit.
    – Mat
    Commented May 22, 2011 at 6:10
  • 7
    @Codemwnci: Not really a good idea if setAge is virtual (as is the case here).
    – user541686
    Commented May 22, 2011 at 6:13
  • @Mehrdad can you explain? why virtual method should not call inside Constructors? Commented Mar 22, 2016 at 1:53
  • 2
    @UnKnown: It is explained here.
    – user541686
    Commented Mar 22, 2016 at 2:12

8 Answers 8

220

Throwing exceptions in a constructor is not bad practice. In fact, it is the only reasonable way for a constructor to indicate that there is a problem; e.g. that the parameters are invalid.

I also think that throwing checked exceptions can be OK1, assuming that the checked exception is 1) declared, 2) specific to the problem you are reporting, and 3) it is reasonable to expect the caller to deal with a checked exception for this2.

However explicitly declaring or throwing java.lang.Exception is almost always bad practice.

You should pick an exception class that matches the exceptional condition that has occurred. If you throw Exception it is difficult for the caller to separate this exception from any number of other possible declared and undeclared exceptions. This makes error recovery difficult, and if the caller chooses to propagate the Exception, the problem just spreads.


1 - Some people may disagree, but IMO there is no substantive difference between this case and the case of throwing exceptions in methods. The standard checked vs unchecked advice applies equally to both cases.
2 - For example, the existing FileInputStream constructors will throw FileNotFoundException if you try to open a file that does not exist. Assuming that it is reasonable for FileNotFoundException to be a checked exception3, then the constructor is the most appropriate place for that exception to be thrown. If we threw the FileNotFoundException the first time that (say) a read or write call was made, that is liable to make application logic more complicated.
3 - Given that this is one of the motivating examples for checked exceptions, if you don't accept this you are basically saying that all exceptions should be unchecked. That is not practical ... if you are going to use Java.


Someone suggested using assert for checking arguments. The problem with this is that checking of assert assertions can be turned on and off via a JVM command-line setting. Using assertions to check internal invariants is OK, but using them to implement argument checking that is specified in your javadoc is not a good idea ... because it means your method will only strictly implement the specification when assertion checking is enabled.

The second problem with assert is that if an assertion fails, then AssertionError will be thrown. Received wisdom is that it is a bad idea to attempt to catch Error and any of its subtypes. But irrespective of that, you are still throwing an exception, albeit in an indirect way.

0
43

I've always considered throwing checked exceptions in the constructor to be bad practice, or at least something that should be avoided.

The reason for this is that you cannot do this :

private SomeObject foo = new SomeObject();

Instead you must do this :

private SomeObject foo;
public MyObject() {
    try {
        foo = new SomeObject()
    } Catch(PointlessCheckedException e) {
       throw new RuntimeException("ahhg",e);
    }
}

At the point when I'm constructing SomeObject I know what it's parameters are so why should I be expected to wrap it in a try catch? Ahh you say but if I'm constructing an object from dynamic parameters I don't know if they're valid or not. Well, you could... validate the parameters before passing them to the constructor. That would be good practice. And if all you're concerned about is whether the parameters are valid then you can use IllegalArgumentException.

So instead of throwing checked exceptions just do

public SomeObject(final String param) {
    if (param==null) throw new NullPointerException("please stop");
    if (param.length()==0) throw new IllegalArgumentException("no really, please stop");
}

Of course there are cases where it might just be reasonable to throw a checked exception

public SomeObject() {
    if (todayIsWednesday) throw new YouKnowYouCannotDoThisOnAWednesday();
}

But how often is that likely?

7
  • 7
    You are not always the producer and consumer of a class. That is, someone else may be using your class without checking. You can easily argue that they've failed to meet your preconditions for using the class and GIGO but the OP asked about the practice being good or bad. I'd argue that making your class easier to use and more reliable is good practice and being liberal in what you accept as inputs helps achieve that.
    – Stephen
    Commented Jul 22, 2015 at 15:40
  • 6
    I think this comes down to the greater debate about checked and unchecked exceptions. Checked exceptions are often mis-used as a kind of response rather than an indication of exceptional circumstances. Few things are more exceptional than an error in the constructor. I think throwing a runtime exception if absolutely necessary is better than trying to indicate to the user that some forseen event occured. If it can be forseen, it's not exceptional is it? Generally I find you can use either IllegalStateException, NullPointerException or IllegalArgumentException.
    – Richard
    Commented Jul 24, 2015 at 12:02
  • I was under the impression the OP was asking about exceptions generally. I agree with your point that most cases are handled by the 3 specific exceptions you refer to.
    – Stephen
    Commented Jul 26, 2015 at 20:06
  • I think his example shows checked exceptions, certainly my answer was regarding checked exceptions.
    – Richard
    Commented Jul 27, 2015 at 14:53
  • 2
    This is a good example of how not to do. Constructor is the only one place, where all checks should be implemented. Entity can't exist without some property. If entity may have null value - it's different business entity or DTO object (another story). Exceptions should always be thrown from constructor to restrict creation of invalid arguments with unexpected NullReferenceExceptions in runtime.
    – Artem A
    Commented Dec 23, 2020 at 13:22
36

As mentioned in another answer here, in Guideline 7-3 of the Java Secure Coding Guidelines, throwing an exception in the constructor of a non-final class opens a potential attack vector:

Guideline 7-3 / OBJECT-3: Defend against partially initialized instances of non-final classes When a constructor in a non-final class throws an exception, attackers can attempt to gain access to partially initialized instances of that class. Ensure that a non-final class remains totally unusable until its constructor completes successfully.

From JDK 6 on, construction of a subclassable class can be prevented by throwing an exception before the Object constructor completes. To do this, perform the checks in an expression that is evaluated in a call to this() or super().

    // non-final java.lang.ClassLoader
    public abstract class ClassLoader {
        protected ClassLoader() {
            this(securityManagerCheck());
        }
        private ClassLoader(Void ignored) {
            // ... continue initialization ...
        }
        private static Void securityManagerCheck() {
            SecurityManager security = System.getSecurityManager();
            if (security != null) {
                security.checkCreateClassLoader();
            }
            return null;
        }
    }

For compatibility with older releases, a potential solution involves the use of an initialized flag. Set the flag as the last operation in a constructor before returning successfully. All methods providing a gateway to sensitive operations must first consult the flag before proceeding:

    public abstract class ClassLoader {

        private volatile boolean initialized;

        protected ClassLoader() {
            // permission needed to create ClassLoader
            securityManagerCheck();
            init();

            // Last action of constructor.
            this.initialized = true;
        }
        protected final Class defineClass(...) {
            checkInitialized();

            // regular logic follows
            ...
        }

        private void checkInitialized() {
            if (!initialized) {
                throw new SecurityException(
                    "NonFinal not initialized"
                );
            }
        }
    }

Furthermore, any security-sensitive uses of such classes should check the state of the initialization flag. In the case of ClassLoader construction, it should check that its parent class loader is initialized.

Partially initialized instances of a non-final class can be accessed via a finalizer attack. The attacker overrides the protected finalize method in a subclass and attempts to create a new instance of that subclass. This attempt fails (in the above example, the SecurityManager check in ClassLoader's constructor throws a security exception), but the attacker simply ignores any exception and waits for the virtual machine to perform finalization on the partially initialized object. When that occurs the malicious finalize method implementation is invoked, giving the attacker access to this, a reference to the object being finalized. Although the object is only partially initialized, the attacker can still invoke methods on it, thereby circumventing the SecurityManager check. While the initialized flag does not prevent access to the partially initialized object, it does prevent methods on that object from doing anything useful for the attacker.

Use of an initialized flag, while secure, can be cumbersome. Simply ensuring that all fields in a public non-final class contain a safe value (such as null) until object initialization completes successfully can represent a reasonable alternative in classes that are not security-sensitive.

A more robust, but also more verbose, approach is to use a "pointer to implementation" (or "pimpl"). The core of the class is moved into a non-public class with the interface class forwarding method calls. Any attempts to use the class before it is fully initialized will result in a NullPointerException. This approach is also good for dealing with clone and deserialization attacks.

    public abstract class ClassLoader {

        private final ClassLoaderImpl impl;

        protected ClassLoader() {
            this.impl = new ClassLoaderImpl();
        }
        protected final Class defineClass(...) {
            return impl.defineClass(...);
        }
    }

    /* pp */ class ClassLoaderImpl {
        /* pp */ ClassLoaderImpl() {
            // permission needed to create ClassLoader
            securityManagerCheck();
            init();
        }

        /* pp */ Class defineClass(...) {
            // regular logic follows
            ...
        }
    }
9
  • 3
    I wonder if anyone else realized the importance of this subtle point. This is something to be aware of when deciding to throw an exception from a constructor. Commented Mar 4, 2015 at 23:26
  • 4
    As I commented on my answer, most Java code does not need to deal with this kind of "attack". It is only relevant if there is the possibility of running untrusted code in a security sensitive context.
    – Stephen C
    Commented Jan 6, 2017 at 7:49
  • 2
    This is only a concern in very limited situations where a partially initialized class can be a security concern. I do not believe it would be suggested to apply this advice to all classes.
    – Mike
    Commented Mar 16, 2017 at 20:12
  • 4
    Post construction validation allows for objects to exist that have not been validated and can therefore exist in invalid states. I view this as a far greater issue in most scenarios. I'm not sure why throwing an exception from a constructor wouldn't integrate well with Spring.
    – Mike
    Commented Mar 17, 2017 at 15:32
  • 2
    @Mike Please don't read more into my comment than what I stated. I never said throwing an exception from a constrictor does not integrate well with Spring. Spring provides the capabilities for post construction validation, which following such a pattern alleviates the need for throwing exceptions from the constructor.
    – Hazok
    Commented Mar 17, 2017 at 15:45
16

You do not need to throw a checked exception. This is a bug within the control of the program, so you want to throw an unchecked exception. Use one of the unchecked exceptions already provided by the Java language, such as IllegalArgumentException, IllegalStateException or NullPointerException.

You may also want to get rid of the setter. You've already provided a way to initiate age through the constructor. Does it need to be updated once instantiated? If not, skip the setter. A good rule, do not make things more public than necessary. Start with private or default, and secure your data with final. Now everyone knows that Person has been constructed properly, and is immutable. It can be used with confidence.

Most likely this is what you really need:

class Person { 

  private final int age;   

  Person(int age) {    

    if (age < 0) 
       throw new IllegalArgumentException("age less than zero: " + age); 

    this.age = age;   
  }

  // setter removed
0
8

This is totally valid, I do it all the time. I usually use IllegalArguemntException if it is a result of parameter checking.

In this case I wouldn't suggest asserts because they are turned off in a deployment build and you always want to stop this from happening, but they are valid if your group does ALL it's testing with asserts turned on and you think the chance of missing a parameter problem at runtime is more acceptable than throwing an exception that is maybe more likely to cause a runtime crash.

Also, an assert would be more difficult for the caller to trap, this is easy.

You probably want to list it as a "throws" in your method's javadocs along with the reason so that callers aren't surprised.

7

I have never considered it to be a bad practice to throw an exception in the constructor. When the class is designed, you have a certain idea in mind of what the structure for that class should be. If someone else has a different idea and tries to execute that idea, then you should error accordingly, giving the user feedback on what the error is. In your case, you might consider something like

if (age < 0) throw new NegativeAgeException("The person you attempted " +
                       "to construct must be given a positive age.");

where NegativeAgeException is an exception class that you constructed yourself, possibly extending another exception like IndexOutOfBoundsException or something similar.

Assertions don't exactly seem to be the way to go, either, since you're not trying to discover bugs in your code. I would say terminating with an exception is absolutely the right thing to do here.

6

I am not for throwing Exceptions in the constructor since I am considering this as non-clean. There are several reasons for my opinion.

  1. As Richard mentioned you cannot initialize an instance in an easy manner. Especially in tests it is really annoying to build a test-wide object only by surrounding it in a try-catch during initialization.

  2. Constructors should be logic-free. There is no reason at all to encapsulate logic in a constructor, since you are always aiming for the Separation of Concerns and Single Responsibility Principle. Since the concern of the constructor is to "construct an object" it should not encapsulate any exception handling if following this approach.

  3. It smells like bad design. Imho if I am forced to do exception handling in the constructor I am at first asking myself if I have any design frauds in my class. It is necessary sometimes, but then I outsource this to a builder or factory to keep the constructor as simple as possible.

So if it is necessary to do some exception handling in the constructor, why would you not outsource this logic to a Builder of Factory? It might be a few more lines of code but gives you the freedom to implement a far more robust and well suited exception handling since you can outsource the logic for the exception handling even more and are not sticked to the constructor, which will encapsulate too much logic. And the client does not need to know anything about your constructing logic if you delegate the exception handling properly.

2
  • 1
    Well, that's only true if you are using Clean Code Architecture and maybe another Domain Driven Design approach. It's important to say that not all softwares should follow Clean Code/DDD principles and the author didn't mentioned using it. Commented Oct 11, 2022 at 17:15
  • If part of the definition of the object is that it has certain constraints, and the provided input parameters violate said constraints, then you cannot reasonably separate "constructing an object" from performing "logic" Commented Jun 4 at 5:17
4

It is bad practice to throw Exception, as that requires anyone who calls your constructor to catch Exception which is a bad practice.

It is a good idea to have a constructor (or any method) throw an exception, generally speaking IllegalArgumentException, which is unchecked, and thus the compiler doesn't force you to catch it.

You should throw checked exceptions (things that extend from Exception, but not RuntimeException) if you want the caller to catch it.

2
  • 1
    This idea that the compiler "forces you to catch things" is misleading. Often you can just declare your method to throw the same exception - right back up to the main method - which means you aren't forced to catch things. The only case in which you are literally forced by the compiler to catch things is when you are overriding a method that doesn't declare that exception, or a superclass of it, in a throws clause. Commented May 22, 2011 at 6:50
  • 2
    Sorry, should have written "forced to deal with" which means catch or add a throws clause.
    – TofuBeer
    Commented May 22, 2011 at 16:33

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