Reputation: 557
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
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
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
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
Reputation: 425258
You have a few choices:
null
. Do this if the user could reasonably enter any indexUpvotes: 1
Reputation: 49804
Don't listen to Eclipse.
You have two choices and both of them can be good or bad, depending on the circumstances.
null
.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 RuntimeException
s for normal program behaviour.
Upvotes: 2
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
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
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