Proper use of an ArgumentException?

Question

From what I saw, ArgumentExceptions are commonly used:

 public void UpdateUser(User user) { if (user == null) throw new ArgumentException("user"); // etc... } 

but what if I have something like this:

 public void UpdateUser(int idOfUser) { var user = GetUserById(idOfUser); if (user == null) throw new ArgumentException("idOfUser"); // etc... } 

Is this another ArgumentException ?

Answer 1

As the name implies, ArgumentException is an exception to the argument. This means that the argument was somehow wrong.

General form:

 public void SomeMethod(SomeType arg) { if(!TestArgValid(arg)) throw new ArgumentException("arg"); //Or more specific is possible //eg ArgumentNullException /* Actually do stuff */ } 

If the only possible way that GetUserById could fail was that something with the idOfUser value was essentially wrong, then the following would be the same in practice:

 public void UpdateUser(int idOfUser) { if(!TestValid(idOfUser)) throw new ArgumentException("idOfUser"); var user = GetUserById(idOfUser); // Do stuff with user } public void UpdateUser(int idOfUser) { var user = GetUserById(idOfUser); if(user == null) throw new ArgumentException("idOfUser"); // Do stuff with user } 

And , if for some reason it turned out to be faster or less wasteful due to some resource for testing user after the fact than idOfUser before the fact and if > there were no side effects of calling GetUserById , and if the difference mattered, the second version is possible would be a reasonable optimization of the first.

But this is only true if all of the above ifs characters are saved, and then this is a strange way to detect an invalid argument, which has a definite advantage when we benefit from encapsulating methods, hiding this oddity from everything else.

Most likely, there may be a valid idOfUser for which there was no corresponding user , in which case, of course, it was no exception.

Answer 2

First

 if (user == null) throw new ArgumentException("user"); 

it should be

 if (user == null) throw new ArgumentNullException("user"); 

If possible, you should not throw an ArgumentException directly

The primary derived classes of ArgumentException are ArgumentNullException and ArgumentOutOfRangeException . These derived classes should be used in place of ArgumentException , except in situations where none of the derived classes is acceptable.

For a second example here, Should I throw a KeyNotFoundException to search the database? they suggest (in the comments)

 if (user == null) throw new ObjectNotFoundException(); 

Defined by System.Data : System.Data.ObjectNotFoundException .