What is the preferred Throwable to use in a private utility class constructor?
Asked Answered
R

8

35

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.

Rodrigues answered 29/12, 2008 at 22:44 Comment(1)
I've put a (rather large) bounty on the following question which asks itself if the AssertionError should actually be thrown here, or if an assert false statement should be used instead.Hyperborean
D
42

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

Ducal answered 29/12, 2008 at 22:46 Comment(8)
Thanks. I guess I've just been looking at AssertionError the wrong way.Rodrigues
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.Dx
@Charlie: surely the stack trace is indication enough? Plus an Exception will get caught by catch (Exception e), an error (rightly) won't.Mouseear
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. :-)Ducal
Incorrect, see the javadoc: java.sun.com/j2se/1.4.2/docs/api/java/lang/AssertionError.htmlDx
@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"Outlier
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.Dx
Link @CharlieMartin has here appears unreachable. Try this docs.oracle.com/javase/10/docs/api/java/lang/…Lipkin
O
12

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");
}
Outlier answered 30/12, 2008 at 17:26 Comment(1)
He is asking why is the type of AssertionError and not of something else?Sectarianism
P
6

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.

Picky answered 29/12, 2008 at 23:43 Comment(2)
Hehe, declare it "throws Throwable", and throw an AssertionError anyway. :-)Ducal
@ChrisJester-Young, No, if you declare it "throws Throwable", then a better implementation is to throw another exception which has a cause of AssertionError.Sectarianism
I
2

What about IllegalAcessError ? :)

Inhabitant answered 29/12, 2008 at 22:50 Comment(2)
I'm not sure if I like that one, since it's a subclass of IncompatibleClassChangeError. But it sounds nice. :)Rodrigues
"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."Hyperborean
D
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.

Dx answered 29/12, 2008 at 23:30 Comment(19)
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.Ducal
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.Ducal
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.Dx
-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.Mouseear
-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.Outlier
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.Dx
And because (2) an assertion error is defined, quote, "Thrown to indicate that an assertion has failed." java.sun.com/j2se/1.4.2/docs/api/java/lang/AssertionError.html . Not "thrown when J Random Programmer thinks a bad thing happened."Dx
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.Outlier
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.Outlier
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.Dx
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.Outlier
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.Thereabouts
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.Thereabouts
@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. ...Dx
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.;Dx
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.Thereabouts
Which attitude is why java exceptions turn out to be so often useless or nearly so in the field.Dx
@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.Sectarianism
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.Dx
A
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.");
}
Anapest answered 15/7, 2018 at 14:15 Comment(1)
UnsupportedOperationException seems to be the most descriptive one for me.Lyric
F
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?

Flatulent answered 29/12, 2008 at 22:48 Comment(2)
My understanding is that this utility class provides a number of static methods only, so the constructor will not be called.Ghat
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.Rodrigues
M
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();
    }

    ...
}
Molluscoid answered 24/1, 2019 at 19:54 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.