34

Effective Java (Second Edition), Item 4, discusses using private constructors to enforce noninstantiability. Here's the code sample from the book:

public final class UtilityClass {
    private UtilityClass() {
        throw new AssertionError();
    }
}

However, AssertionError doesn't seem like the right thing to throw. Nothing is being "asserted", which is how the API defines the use of AssertionError.

Is there a different Throwable that's typically in this situation? Does one usually just throw a general Exception with a message? Or is it common to write a custom Exception for this?

It's pretty trivial, but more than anything I guess I'm just curious about it from a style and standards perspective.

Rob Hruska
  • 118,520
  • 32
  • 167
  • 192
  • 2
    I've put a (rather large) bounty on the [following question](http://stackoverflow.com/q/22053306/589259) which asks itself if the `AssertionError` should actually be thrown here, or if an `assert false` statement should be used instead. – Maarten Bodewes Aug 01 '14 at 18:57

8 Answers8

42

There is an assertion: "I'm asserting that this constructor will never be called". So, indeed, AssertionError is correct here.

C. K. Young
  • 219,335
  • 46
  • 382
  • 435
  • Thanks. I guess I've just been looking at AssertionError the wrong way. – Rob Hruska Dec 29 '08 at 23:04
  • 1
    Strongly disagree here, see below. You need to be able to find your way to the place where the exception started; you also should make the exception give you, or a naive reader of the code, a good hint what happened. An AssertionError not associated with an assertion does neither. – Charlie Martin Dec 29 '08 at 23:56
  • @Charlie: surely the stack trace is indication enough? Plus an Exception will get caught by catch (Exception e), an error (rightly) won't. – cletus Dec 30 '08 at 00:54
  • I agree with cletus's comments here, the stacktrace is invaluable, and exception chaining is the best feature since sliced bread, because it allows you to preserve the stack trace far and wide. :-) – C. K. Young Dec 30 '08 at 02:05
  • 2
    Incorrect, see the javadoc: http://java.sun.com/j2se/1.4.2/docs/api/java/lang/AssertionError.html – Charlie Martin Dec 30 '08 at 18:07
  • 2
    @Charlie the javadoc says "Thrown to indicate that an assertion has failed." You are looking at an assertion as "using the assert keyword" instead of "the programmer regards this as a bug" – Craig P. Motlin Dec 30 '08 at 18:36
  • 2
    You're right, I am. That's why it's called the "assert" statement. Just like I wouldn't use a NullPointerException to indicate a name that has value 0 instead of a number in range, even if the number was meant as an index. – Charlie Martin Jan 04 '09 at 02:49
  • 1
    Link @CharlieMartin has here appears unreachable. Try this https://docs.oracle.com/javase/10/docs/api/java/lang/AssertionError.html – Dut A. Oct 30 '21 at 21:39
11

I like including Bloch's comment:

// Suppress default constructor for noninstantiability

Or better yet putting it in the Error:

private UtilityClass()
{
    throw new AssertionError("Suppress default constructor for noninstantiability");
}
Craig P. Motlin
  • 26,452
  • 17
  • 99
  • 126
5

UnsupportedOperationException sounds like the best fit, though a checked exception would be even better, since it might warn someone erroneously instantiating the class at compile time.

Michael Borgwardt
  • 342,105
  • 78
  • 482
  • 720
  • 3
    Hehe, declare it "throws Throwable", and throw an AssertionError anyway. :-) – C. K. Young Dec 29 '08 at 23:47
  • 1
    @ChrisJester-Young, No, if you declare it "throws Throwable", then a better implementation is to throw *another* exception which has a `cause` of AssertionError. – Pacerier Oct 31 '14 at 22:30
2

What about IllegalAcessError ? :)

OscarRyz
  • 196,001
  • 113
  • 385
  • 569
  • I'm not sure if I like that one, since it's a subclass of IncompatibleClassChangeError. But it sounds nice. :) – Rob Hruska Dec 29 '08 at 22:55
  • "Normally, this error is caught by the compiler; this error can only occur at run time if the definition of a class has incompatibly changed." – Maarten Bodewes Aug 01 '14 at 18:58
2

No no no, with all due respect to Josh Bloch, never throw an AssertionError unless it's from an assertion. If you want an AssertionError here, throw it with assert(false). Then someone reading the code can find it later.

Even better, define your own exception, say CantInstantiateUtilityClass. then you'll have code that says

try {
    // some stuff
} catch (CantInstantiateUtilityClass e) {
    // react
}

so that the reader of the catcher knows what happened.


Let me just note that the standard still defines AssertionError as the result of a failed assertion, not as what some beginner thinks ought to be thrown in place of a well-defined informative exception. Sadly, good exception discipline is perhaps the least encouraged skill in Java programming.

E_net4
  • 27,810
  • 13
  • 101
  • 139
Charlie Martin
  • 110,348
  • 25
  • 193
  • 263
  • 7
    _Strongly_ disagree. (Bloch, not Block, by the way.) The problem with "assert false" is that if you have assertions turned off, the assertion will fail to be thrown. People should find assertions by looking for AssertionError as well as assert. – C. K. Young Dec 29 '08 at 23:35
  • I also strongly disagree with CantInstantiateUtilityClass as an exception type: first, it doesn't contain the word Exception or Error at the end, and second, AssertionError allows you to give an error message, and that should be used in terms of hinting people as to what happened. – C. K. Young Dec 29 '08 at 23:36
  • 1
    Dammit, I've known Josh for ten years, you'd think I could spell his name. Okay, so tag Exception on if you like; but it shouldn't be an assertion error unless it's caused by an assertion. Someone new reading the code won't have a clue, and your example doesn't offer one. – Charlie Martin Dec 29 '08 at 23:54
  • 6
    -1 An exception will get caught by some (possibly random) catch (Exception e) block. An Error won't be. Creating a custom exception for something that can only happen by modifying the source code or having a custom ClassLoader is terrible advice. – cletus Dec 30 '08 at 00:55
  • 1
    -1 Why should your code have to catch CantInstantiateUtilityClass? This is something that should never happen. Which is exactly when assertions are appropriate. Others have pointed out the problem with the assert keyword. – Craig P. Motlin Dec 30 '08 at 17:28
  • 1
    Because (1) it's undesirable for a program to handle errors by giving up and dying. Either this program is catching AssertionError -- which obviates your objection -- or it handles the error by abnormal termination. – Charlie Martin Dec 30 '08 at 18:03
  • 1
    And because (2) an assertion error is defined, quote, "Thrown to indicate that an assertion has failed." http://java.sun.com/j2se/1.4.2/docs/api/java/lang/AssertionError.html . Not "thrown when J Random Programmer thinks a bad thing happened." – Charlie Martin Dec 30 '08 at 18:06
  • 1
    Calling the constructor is a bug, so we should assert that it is never called. The assert keyword is deeply flawed and should never be used. That makes "throw new AssertionError()" the new assertion mechanism. – Craig P. Motlin Dec 30 '08 at 18:25
  • 2
    I disagree, the program should give up and die if the constructor is called, since it is obviously a bug. assert(false) will also cause the program to give up and die, also with an AssertionError, so I'm not sure why you prefer the assert keyword. – Craig P. Motlin Dec 30 '08 at 18:28
  • 2
    I'm sure you'll be just thrilled when your airplane control system gives up and dies instead of trying to recover. Abnormal termination is not a a reliability strategy. – Charlie Martin Jan 04 '09 at 02:46
  • Far worse for the airplane system to muddle on in an inconsistent state no programmer considered possible. Better to give up and let the pilots take over. :) Seriously though, disabling assertions is fine if you work on pacemakers and such. Everyone else should avoid it. – Craig P. Motlin Jan 05 '09 at 17:44
  • 3
    About your update: The actual text is `Thrown to indicate that an assertion has failed.` nowhere does it state WHOSE assertion we're talking about - at least I **DO** have assertions about my code, you may not.. But then the whole thing is nitpicking anyhow - nobody would ever misinterpret such an exception with a good cause ("May not instantiate instance of class X") and a stack trace pointing to the actual constructor somehow wrongly. – Voo Nov 13 '11 at 16:43
  • 1
    cont. Also since you're asking.. no, no I really wouldn't feel any safer with a program that thought good error handling meant silently ignoring every exception and just continued silently corrupting state - and I'd take any bet that that's not how these things are implemented in airplane software.. I'd think catching all errors in some well defined points in the program and restarting from there would make much more sense than that. – Voo Nov 13 '11 at 16:46
  • 1
    @Voo, if you think that's what I suggested, you're a dope. Of course you then have to do something *with* the assertion. But an uncaught exception does NOTHING except cause the program to fail. Now, in erlang, where this is a well-understood mechanism supported by the language, you can fail and expect a new process to restart. In Java, where, uncaught, the exception simply kills the JVM, this isn't an option. Think of it this way: if you define an exception appropriate to the domain, you have the information that makes it possible to recover. ... – Charlie Martin Nov 14 '11 at 04:44
  • 1
    Use an AssertionError, and you just know an assertion failed -- and at that, using `throw new AssertionError`, the code's lying to you even then. Now consider how nonplussed a programmer will be to find `AssertionErrors` coming out of a program that has assertions *turned off*? This approach (1) lies to the runtime system, (2) lies to the maintenance programmer, and (3) makes it appear there's a JVM bug when assertions are turned off.; – Charlie Martin Nov 14 '11 at 04:46
  • 1
    Yeah it seems nigh impossible to figure out what the AssertionError with cause `You may not instantiate class X.` and a stack trace pointing to the constructor of class X could possibly mean. Now about HOW you catch an error and restart is a topic on its own and in that case it may make sense to define unique exceptions for every error case, but that's hardly important for say 99.99% of all programmers (i.e. would also apply to every other existing exception in java) - also there are other ways to solve this problem as well. – Voo Nov 14 '11 at 14:43
  • Which attitude is why java exceptions turn out to be so often useless or nearly so in the field. – Charlie Martin Nov 14 '11 at 17:05
  • @ChrisJester-Young, You stated that "people should find assertions by looking for `AssertionError` as well as `assert`". Where did you get that from? Is there a citation/source or is it merely an assumption? Because the inverse assumption that people/parsers should find assertions by looking for **only** `assert` is an equally-if-not-more valid default stand. – Pacerier Oct 31 '14 at 22:57
  • 1
    It comes from, at this point, just short of 20 years experience using the language, as a Senior Java Architect at Sun, and as the author of a fair bit of Sun's Java certification tests and Sun's first J2EE Architecture course. I didn't cite another authority because I *am* an authority. – Charlie Martin Nov 02 '14 at 18:32
2

When the code requires the inclusion of the JUnit as a dependency such as within the maven test scope <scope>test</scope>, then go straight to Assertion.fail() method and benefit from significant improvement in clarity.

public final class UtilityClass {
    private UtilityClass() {
        fail("The UtilityClass methods should be accessed statically");
    }
}

When outside the test scope, you could use something like the following, which would require a static import to use like above. import static pkg.Error.fail;

public class Error {
    private static final Logger LOG = LoggerFactory.getLogger(Error.class);
    public static void fail(final String message) {
        LOG.error(message);
        throw new AssertionError(message);
        // or use your preferred exception 
        // e.g InstantiationException
    }
}

Which the following usage.

public class UtilityClassTwo {
    private UtilityClassTwo() {
        Error.fail("The UtilityClass methods should be accessed statically");
    }
}

In its most idiomatic form, they all boil down to this:

public class UtilityClassThree {
    private UtilityClassThree() {
        assert false : "The UtilityClass methods should be accessed statically";
    }
}

One of the built in exceptions, UnsupportedOperationException can be thrown to indicate that 'the requested operation is not supported'.

 private Constructor() {
    throw new UnsupportedOperationException(
            "Do not instantiate this class, use statically.");
}
Martin Spamer
  • 5,437
  • 28
  • 44
0

A broken assertion means that you've broken a contract specification of your code. So it's the right thing here.

However, as I assume you'll be privately instantiating an instance, it will also call the constructor and cause an error- unless you have another constructor?

Steve B.
  • 55,454
  • 12
  • 93
  • 132
  • My understanding is that this utility class provides a number of static methods only, so the constructor will not be called. – Matthew Murdoch Dec 29 '08 at 22:54
  • Matthew is correct. Technically nothing even needs to be thrown since the constructor's private. Throwing something insures that the class itself doesn't call the constructor. – Rob Hruska Dec 29 '08 at 22:57
0

You can create your own class extending Throwable, e.g.:

class NoninstantiabilityError extends Throwable

This has the following advantages:

  • The name indicates the problem
  • Because it directly extends Throwable it is unlikely that it will be caught by accident
  • Because it directly extends Throwable it is checked and calling the respective constructor by accident would require catching the exception

Usage example:

public final class UtilityClass {
    private UtilityClass() throws NoninstantiabilityError {
        throw new NoninstantiabilityError();
    }

    ...
}
Marcono1234
  • 5,856
  • 1
  • 25
  • 43