javaexceptionillegalstateexceptionruntimeexception

What exception to throw if an operation would bring the object into an illegal state?

Consider a Product with a quantity which can be increased and decreased by a given amount. The quantity should never become negative and if it is going to happen, the operation must be prohibited and the user warned.

public class Product{
    
    private int quantity;
    
    public Product() {
        quantity = 10;
    }
    
    public void decreaseQuantity(int amount) {
        int decreasedQuantity = quantity - amount;
        if(decreasedQuantity < 0 )
            throw new RuntimeException(String.format("Decrease quantity (%s) exceeds avaiable quantity (%s)",
                    amount, quantity));
        
        quantity = decreasedQuantity;
    }
}

For example if a product has quantity 10 and I try to remove 20, I throw a RuntimeException. SonarCloud suggests to replace the RuntimeException with a Custom exception, but I was wondering if there is a standard exception suitable for this case (Effective Java: Favor The Use of Standard Exceptions).

The most suitable exception seems to be IllegalStateException. From the javadoc

Signals that a method has been invoked at an illegal or inappropriate time. In other words, the Java environment or Java application is not in an appropriate state for the requested operation.

and from Effective Java

IllegalStateException: This exception is used if, when a method is called, the state of the object is not valid for that operation. Potentially you have a filehandle and you call read before it has been opened.

However, it seems to me that there is a subtle difference between my example and what is assumed in the docs: it is not the state of the object itself that makes the operation illegal, it's the state of the object and the value of the input parameter. Reading usage examples (like What's the intended use of IllegalStateException?) the object is always in a state where the operation is rejected regardless of the input parameter.

Solution

  • There is nothing in the core java libraries that is a slam dunk. However, make your own exception is probably your best bet, it's a close race between that and using IllegalArgumentException.

    RuntimeException

    Ill advised for nebulous reasons. The primary reason not to use it, is because linter tools will yell at you. The reason they yell at you is twofold, and to determine whether the right move is to tell the linters to shut up, or to head their advice, is to focus on these 2 reasons:

    1. The type name of the exception is itself information. For example, throw new NullPointerException("x is null") is dumb code to write (even though its common). It's superfluous - just new NullPointerException("x") is appropriate. RuntimeException conveys almost no information at all. You are mostly avoiding this trap: Whilst RuntimeException indeed conveys nearly nothing, the message of the exception says it all, therefore, the thing the linter is trying to stop from happening (throwing an exception that doesn't properly convey the nature of the problem) isn't happening, and thus you should consider just telling the linter to stop whining... except for:

    2. The second reason you want a proper exception type is that you really, really don't want code that intends to catch this to have to do string analysis on an exception message. Thus, if there's any world in which you can foresee that some code wants to invoke your method and then react to the condition (of trying to subtract more than available) in any way other than blowing up all context, then you should be throwing an exception that means this specific condition and nothing else. RuntimeException is never appropriate to catch if your intent is to catch a specific condition (it's pretty much never appropriate, period - sometimes you want to run code and react to any problem regardless of its nature, but then catch (Exception e) is the appropriate catch block, or even Throwable. e.g. appservers and the like should be doing this.

    IllegalArgumentException

    The linters won't be yelling, but you're still not really getting the secondary benefit (that is, allow callers to catch this specific problem and not some other problem with the arguments). It's also 'mentally' slightly dubious: Your reasons for initially disregarding it are not wrong. Generally IAEs are understood to imply that the illegal argument is illegal regardless of the state of this object. But this isn't written in IAE's javadoc and isn't universally applied.

    So, downsides are:

    Upside is simply:

    IllegalStateException

    I think this is strictly worse than IAE. It's mostly the same story, except here the confusion is that ISE is usually used to flag that the current state of the object is such that the operation you are trying to call is simply not available. In a tweaked way that's actually true (the object's state is that there are 3 items; therefore it is not in a state that decreaseQuantity(5) is an available operation right now), but it feels more confusing than IAE: ISE feels like the Product object itself is in an invalid state. I'd be assuming that the product is a legacy product that is not now or will ever be in stock again, or is some dummy product type (a Product object representing 'unknown product' or something similarly exotic).

    But, same deal with IAE: The javadoc of ISE doesn't specifically spell out that you couldn't do it. Hence, if you would prefer to throw this, you could, it's not provably incorrect, at worst it's simply bad code style. A linter tool is never going to fault you for it, or if it does, the linter tool is wrong.

    write your own

    Advantages:

    try {
  product.decreaseQuantity(5);
} catch (QuantityInsufficientException e) {
  ui.showOrderingError("Regretfully we don't have this item in stock at the quantity you want");
}

    Is slightly more readable than catch (IllegalArgumentException), and more importantly, is more reliable: IAE is such an oft-used exception that you're going to one day edit the decreaseQuantity method and introduce a code path that throws IAE for some other reason and now you have a very hard to find bug.

    Conclusion

    I'd be writing your own. Yes, it's a bit of a drag to have to write the source file, but you can let your IDE (or Project Lombok for maximum boilerplate busting) generate the entire file and you probably never have to look at InsufficientQuantityException.java ever again.