Should I throw IllegalArgmentException when 'user of given id not exist'?

Question

Say we have a method changeUserName(Long id,String newName) which invokes the repository's findUser(Long id) to find the right user entity and then change its name.
Is it appropriate to thow an IllegalArgmentException when findUser returns null ?
Or should I instead throw a custom UserNotExistException (extends AppException extends RuntimeException) ?

UPDATE:

• RuntimeException:
  @nachokk @JunedAhsan Actually I deliberately make all the exceptions unchecked , because I think this way makes client code clean , easy for debuging and more safe. As to those "unhandled" ones, I'll catch them all on the top of layers thus avoiding showing them on the UI.
  This is due to the fact that many clients catch checked exceptions and then just ignore it, and in some cases they don't know how to handle it. This is a hidden trouble.

• Clarification:
  Sorry for my bad English. What I meant is if the changeUserName should throw an IllegalArgumentException, not the findUser method. And another question: how to differentiate illegal argument from business rule violation?

Answer 1

You should use UserNotExistException. The name is very declarative of what is happening. In my opinion you should to avoid returning null but if you do you have to document it.

UPDATE

I was thinking and as @JunedAhsan suggest, UserNotExistException could be better a CheckedException (extends from Exception and not RuntimeException).

From this link: Unchecked Exceptions : Controversy

If a client can reasonably be expected to recover from an exception, make it a checked exception. If a client cannot do anything to recover from the exception, make it an unchecked exception.

/**
* @return User found or throw UserNotExistException if is not found
*/
public User findUser(Long id) throws UserNotExistException{
    //some code
    User user = giveMeUserForSomePlace();
    if(user == null){
        throw new UserNotExistException();
    }  
    return user;   
}

Answer 2

It depends on how you handle exceptions.

IllegalArgumentException is ok if you only display error report by using e.getMessage() and you don't care repetitive string appending code.

Here is some advantage I find by using custom exceptions:

1. Reduce reptetive code:

Let's say changeUserName is surely not the only case you'll load User, so this code snippet below will happen everytime you invoke repository.findUser(Long id)

if (user == null) {
    throw new IllegalArgumentException("No such user found with given id["+ userId +"]");
}

On the other hand, an ad-hoc exception is much more handy:

if (user == null) {
     throw new UserNotExistException(userId);
}

public class UserNotExistException extends RuntimeException {
     public UserNotExistException(Long id) {
         super("No such user found with given id["+ id +"]");
     }
}

2. You need more support from your exceptions:

Maybe you need to return status code or something like that. An custom exception hierachy may do some help:

see this answer for detail.

Answer 3

I would too suggest to use UserNotExistException but with a difference that instead of it being unchecked exception (by virtue of extending RuntimeException), make it checked exception (extending Exception if AppException is not doing this already).

This will make sure that caller of changeUserName handles UserNotExistException exception and make the code a bit robust.