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

Question

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.

Answer 1

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:

• It's a bit of a push to force callers to catch (IllegalArgumentException e) to deal with wanting to react to subtracting more than is there. IAE is still too 'general'.
• It could be lightly confusing.

Upside is simply:

• It's already there.

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:

• There's no chance that users of your library bring faulty assumptions (that IAE means: Argument is illegal regardless of state); that makes it less confusing.
• If code intends to call your method and write a catch clause specifically for the condition of removing more than is there, then this is definitely the nicest answer. This:

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.

Answer 2

java.lang.IllegalArgumentException is the right answer in this case.

This example is equivalent to the following one from Effective Java - Third Edition (page 301):

Consider the case of an object representing a deck of cards, and suppose there were a method to deal a hand from the deck that took as an argument the size of the hand. If the caller passed a value larger than the number of cards remaining in the deck, it could be construed as an IllegalArgumentException (the handSize parameter value is too high) or an IllegalStateException (the deck contains too few cards). Under these circumstances, the rule is to throw IllegalStateException if no argument values would have worked, otherwise throw IllegalArgumentException.

TheIllegalStateException is not correct because the state of the object does not prevent to invoke decreaseQuantity: you can invoke it, just use an appropriate input value.