Home Exceptions

You should place your exception checks as early as possible. For example:

public void add(String item) {
    item = item.lower();
    if (item.isEmpty()) {
        throw new IllegalArgumentException("'item' should not be an empty string");
    }
    
    // etc
}

The exception check does not rely on the string being lowercase, so it's safe to swap the two. A corrected version would look like this:

public void add(String item) {
    if (item.isEmpty()) {
        throw new IllegalArgumentException("'item' should not be an empty string");
    }
    item = item.lower();
    
    // etc
}

Sometimes, you'll find that your exception check cannot go literally at the top of your method. That's fine, as long as you make an attempt. For example:

public void sumOfGrades() {
    int sum = 0;
    for (int i = 0; i < size; i++) {
        if (this.data[i] < 0) {
            throw new IllegalStateException("Cannot have a negative grade");
        }
    }
    return sum;
}

Because each exception check that your method performs is completely independent from each other, you should use separate if-statements like so:

public void myMethod(int a, int b) {
    if (a == b) {
        throw new IllegalArgumentException("Params should not be equal");
    }
    if (this.myField.length() > b) {
        throw new IllegalStateException("Internal data has grown too large");
    }

    System.out.println("Rest of method goes here");
}

In contrast, the following version is incorrect – by using else-if, you're implying the exception checks are related and mutually exclusive, which is inaccurate since both of those conditions could actually be false:

public void myMethod(int a, int b) {
    if (a == b) {
        throw new IllegalArgumentException("Params should not be equal");
    } else if (this.myField.length() > b) {
        throw new IllegalStateException("Internal data has grown too large");
    }

    System.out.println("Rest of method goes here");
}

Likewise, placing the body of the method into an else branch would also be incorrect for the same reason: it implies that each of the three options are "equal" in rank, and are mutually exclusive:

public void myMethod(int a, int b) {
    if (a == b) {
        throw new IllegalArgumentException("Params should not be equal");
    } else if (this.myField.length() > b) {
        throw new IllegalStateException("Internal data has grown too large");
    } else {
        System.out.println("Rest of method goes here");
    }
}

For example, take the following snippet, which is missing a custom error message:

if (age < 21) {
    throw new IllegalArgumentException();
}

While we do permit code like this, we discourage it because when you try and run this, and when this exception is triggered, all the client will see is something like this:

Exception in thread "main" java.lang.IllegalArgumentException
  at EventPlanner$GroceryStore.buyAlcohol(EventPlanner.java:140)
  at EventPlanner.purchaseSupplies(EventPlanner.java:30)
  at EventPlanner.main(EventPlanner.java:8)

...which is not particularly helpful. All the client can tell is that the class threw an IllegalArgumentException of some kind, which is not particularly helpful when debugging since it forces them to either have to read your comments or dig through your source code, which is extra work for them.

Instead, it's better to be descriptive and provide a detailed error message such as this:

if (age < 21) {
    throw new IllegalArgumentException(
        "Cannot purchase alcohol if age < 21; currently age = " + age);
}

This way, when this exception is thrown, you get a stack trace which looks like this:

Exception in thread "main" java.lang.IllegalArgumentException: Cannot purchase alcohol if age < 21; currently age = 19
  at EventPlanner$GroceryStore.buyAlcohol(EventPlanner.java:140)
  at EventPlanner.purchaseSupplies(EventPlanner.java:30)
  at EventPlanner.main(EventPlanner.java:8)

This is much more helpful – we can immediately identify what the problem is and how to fix it just from the stack trace alone.

As an example, the following would not be acceptable:

// Pre:  'fuel' is the amount of gas to add to the car, in gallons
//       Throws an exception when fuel is negative or tank is full 
// Post: Fills up the fuel tank by the given amount of fuel
public void addFuel(int fuel) {
    if (fuel < 0) {
        throw new IllegalArgumentException("Cannot add negative amount of fuel");
    }
    if (this.amount == MAX_CAPACITY) {
        throw new IllegalStateBalance("Fuel tank is already full");
    }
    // ...
}

We do not specify exactly what kinds of exceptions we will be throwing. This is impolite to our clients, since they have no way of knowing how exactly to handle our different failure modes.

Instead, do this:

// Pre:  'fuel' is the amount of gas to add to the car, in gallons
//       Throws IllegalArgumentException when fuel is negative
//       Throws IllegalStateException when the tank is already full
// Post: Fills up the fuel tank by the given amount of fuel
public void addFuel(int fuel) {
    if (fuel < 0) {
        throw new IllegalArgumentException("Cannot add negative amount of fuel");
    }
    if (this.amount == MAX_CAPACITY) {
        throw new IllegalStateBalance("Fuel tank is already full");
    }
    // ...
}

Note here we document exactly which exceptions we'll be throwing by name, and under exactly which conditions.

It does not matter if you include this information as a part of your preconditions or postconditions – you could make a reasonable argument that it belongs to either category.