Is IllegalStateException appropriate for an immutable object?

Question

Would you throw an IllegalStateException if:

1. A method is unable to do its job because of the value(s) of one or more fields
2. Those fields are final and assigned only in the constructor?

Textbook example: your class is an immutable Collection<BigInteger> and your method is supposed to return the maximum element, but this instance is empty.

I have read Kevin Bourillon`s blog post on the subject and I am not sure which rule applies.

UnsupportedOperationException - this means that the method invoked will always fail for an instance of this class (concrete type), regardless of how the instance was constructed.

Definitely not. Many instances of this class are not empty and the operation would have succeeded.

IllegalStateException - ... there does exist at least one alternate state that the instance in question could have been in, which would have passed the check ... <snip> ... Note also that this exception is appropriate whether or not it is possible to actually mutate this aspect of the instance's state, or it's already too late.

Not quite. This instance was constructed with zero length, so this instance is not and could never have been non-empty.

IllegalArgumentException - throwing this exception implies that there exists at least one other value for this parameter that would have caused the check in question to pass.

Might apply if the parameter in question is the implicit this parameter. This is the exception I am tempted to throw, but I am concerned that it could be confusing.

---

Update: changed example from Collection<Integer> to Collection<BigInteger> because the fact that there was an identity element (Integer.MIN_VALUE) distracts from the question.

Answer 1

It doesn't sound like any of the common exception classes you mention above fit into the Textbook example. It might be more appropriate to return -1 or something.

Alternatively, you could throw a NoSuchElementException, that's exactly what the Collections max() method does:

http://download.oracle.com/docs/cd/E17476_01/javase/1.5.0/docs/api/java/util/Collections.html#max(java.util.Collection)

Answer 2

I think IllegalStateException is appropriate here. The instance could have been in the correct state, if it was constructed correctly (i.e. the "it's already too late" part).

Answer 3

If the state of the class is valid (empty collection), the max element is just null. If the state is not valid, an IllegalArgumentException should have been thrown at construction time.

Answer 4

IllegalStateException comes closest to what you want: "this exception is appropriate whether or not it is possible to actually mutate this aspect of the instance's state".

Not UnsupportedOperationException, since it might succeed for some instance of that class, and throwing IllegalArgumentException for a method that (presumably) takes no arguments will certainly confuse people.

Answer 5

Is IllegalStateException appropriate for an immutable object?

No, because immutable objects only have one state, and could not have pased from one legal state to another.

So, you're constructing an immutable object, and your object should have a max method

class YourObject {
    public BigInteger max(){ ... }
}

I this case IllegalAgumentException should be the correct, but not until the method is executed, but when the object is created!

So, in this scenario, if you have an immutable collection of bigintegers, and you create it with zero elements you are receiving an "invalid argument" upon the collection creation, that's when you have to throw the exception.

I agree with Jon, if your use case, or in you analysis, you're willing to support the rest of the operations, you could throw NoSuchElementException, but I think that would be to postpone the problem. Better would be to avoid the object creation in first place.

So, throwing IllegalArgumentException would be like:

  // this is my immutable object class
  final class YourObject {
      private final Collection<BigInteger> c;
      public YourObject( BigInteger ... values ) {
          if( values.length == 0 ) { 
              throw new IllegalAgumentException("Must specify at least one value");
          }
          ... initialize the rest... 
      }
      public BigInteger max() {
          // find and return the max will always work 
      }
   } 

Client:

   YourObject o  = new YourObject(); // throws IllegalArgumentException 
   // the rest is not executed....
   doSomething( o ) ; 
   ...
   doSomething( YourObject o ) {
        BigInteger maximum = o.max();
   }

In this case you don't need to check for anything in doSomething because the program would fail at the instance creation, which in turn would be fixed at development time.

Throwing NoSuchElementException would be like:

  final class YourObject {
      private final Collection<BigInteger> c;
      public YourObject( BigInteger ... values ) {
           // not validating the input  :-/ oh oh.. 
          ... initialize the rest... 
      }
      public BigInteger max() {
          if( c.isEmpty() ) { throw NoSuchElementException(); }
          // find and return the max will always work after this line
      }
   } 

Client:

   YourObject o  = new YourObject(); // it says nothing
   doSomething( o ) ;
   ...
   doSomething( YourObject o ) {
        BigInteger maximum = o.max();// ooops!, why? what?...  
        // your object client will start questioning what did I do wrong
        // and chais of if( o != null && o.isEmpty() || moonPhaseIs... ) 
   }

Bear in mind, that, if a program is to fail, the best thing you can do is to making it fail fast.

Collections.max have a different purpose, because being an utility method (not an immutable object ), he cannot held responsibility for the empty collection creation ( he wasn't present when that happened ), the only thing he can do is to say "There is no such a thing as max in this collection" hence NoSuchElementException.

One last remark, RuntimeExceptions, should be used for programming mistakes only ( those that can be fixed by testing the application before releasing it )