user1825441
user1825441

Reputation: 557

What to return on an IndexOutOfBoundsException exception?

I have the following method

private ArrayList<User> allUsers = new ArrayList<User>();

public User getUser(int index) {
    try {
        return allUsers.get(index);
    }
    catch(IndexOutOfBoundsException e) {
        // What should I return here?? Say that you want index 0 and no User
        // exists in the ArrayList allUsers, what should I then return? The
        // method needs a User to be returned 
    }
}

And I'm not sure how to do here, I'm sure thats its an easy fix, but what should I return in the catch block? Eclipse is complaining that a User must be returned.

Upvotes: 4

Views: 10211

Answers (8)

user
user

Reputation: 6947

My general opinion is that you should never catch an exception that you don't know how to handle. Particularly in this case, since IndexOutOfBoundsException is a RuntimeException and thus does not need to be caught - you can just as well let it propagate up the call stack. The caller is asking for an object by list index, so presumably has some idea of which index to ask for -- then, throwing or allowing a thrown IndexOutOfBoundsException to propagate seems perfectly natural.

The only other obvious option would be to swallow the exception and return null, but I really don't like such an approach for such flagrant errors on the part of the caller when there is no sensible return value. You could also return a special User instance (refer null object pattern), but even that does not absolve the caller of the responsibility to check what was returned. Depending on the interface and implementation of User such checks may be trivial or not, but it still needs to be done somewhere.

If you want to be clear that the method can throw an exception, just say so:

public User getUser(int index) throws IndexOutOfBoundsException { ... }

Or like @Bela Vizer suggested, wrap it in an IllegalArgumentException (which also is a RuntimeException).

And as pointed out by @lc., it's better to check first yourself if the object exists before trying to access it. Handle yourself the error case that you expect rather than relying on the get() method call to throw an exception. You should still be clear about the fact that the method might throw such an exception, however, if for example the collection is modified between the check and return. With multithreaded software on multi-core CPUs, stranger things have been known to happen.

Upvotes: 15

Bela Vizer
Bela Vizer

Reputation: 2537

There are many possibilities. Some of them depends on your beliefs.

  • You can throw an IllegalArgumentException since the passed argument is not valid.

  • You can throw the IndexOutOfBoundsException.

  • If you want to make sure clients (caller of this method) have to take care of it, you can even declare a checked exception (define your exception class extending Exception), because IllegalArgumentException and IndexOutOfBoundsException are runtime exception which means you don't have to prepare yourself explicitly.

I usually check if the index is in range, if not return null and mention in the javadoc that it can return null if...

Upvotes: 4

alemangui
alemangui

Reputation: 3655

I would throw an exception if no user is found. This exception is caught when the method is called. You could modify the code here to use a custom exception if you want. Something like:

public User getUser(int index) throws IndexOutOfBoundsException {
    if(index >= allUsers.size()){
        throw new IndexOutOfBoundsException("User doesn't exist");
    }
    return allUsers.get(index);
}

Upvotes: 4

Bohemian
Bohemian

Reputation: 425258

You have a few choices:

  1. Return a null. Do this if the user could reasonably enter any index
  2. Don't catch the exception. Do this if it is totally unexpected that the user could enter bad data, in other words, the input is being checked already so it's a "programming error" (or bug) that caused the situation
  3. Throw a checked exception. Do this if you feel the calling code can and should deal with the problem

Upvotes: 1

biziclop
biziclop

Reputation: 49804

Don't listen to Eclipse.

You have two choices and both of them can be good or bad, depending on the circumstances.

  1. You can return null.
  2. You can just re-throw the exception (or rather, not even catch it at all), asking the calling method to deal with it.

There are several more variations, but the basic choice is between the above two: handle the problem in place or delegate the task to the caller.

From that code alone it's impossible to tell which one is the right solution, only you can know what is more appropriate to the situation.

Whichever choice you take though, it's better to check the index manually (0<=index<allUsers.size()) and not relying on RuntimeExceptions for normal program behaviour.

Upvotes: 2

lc.
lc.

Reputation: 116528

Ask yourself the question: "what should you return if you want index 0 and no User exists?" and return whatever you answer.

If you have no answer, you should either re-throw the exception or not be catching it in the first place.

Note that often times, the answer would be to return null, if it is acceptable behavior to ask for a non-existent User.


Side comment: It is usually considered "good practice" to not rely on catching exceptions, but to test for error conditions first. In your case you are trying to get an invalid object index first then reacting if the getter blows up. Instead, I would recommend to test the index parameter first (make sure it is at least zero and less than the length of allUsers), and if it fails the test to do something (return null or throw your own exception).

Upvotes: 11

danielrvt
danielrvt

Reputation: 10926

I would return null, however, if you feel your app might crash because of that, you can simply return a "nobody" User like this:

return new User("nobody", ...);

And handle that case outside the function.

Another alternative is to throw the exception and handle it outside.

Upvotes: 2

Paul Bellora
Paul Bellora

Reputation: 55233

Assuming index is user input, just let IndexOutOfBoundsException propagate and catch it farther up at a point where you can display an error message.

In fact, you could first validate index against allUsers.size() before even trying the lookup to do the same thing. User input should generally be validated at the earliest point possible.

Upvotes: 2

Related Questions