Correct Usage of ArgumentException?

Question

From what I've seen, ArgumentExceptions are usually used like such:

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 that still an ArgumentException?

For the second one, [here](http://stackoverflow.com/q/21146594/613130) (in comments) they suggest `ObjectNotFoundException` — xanatos, Jun 22 '15 at 13:18
If anything, the 2nd example is more of an ArgumentException then the 1st one (which should be ArgumentNullException) — H H, Jun 22 '15 at 13:22
In your 2nd scenario, I can only think of *one* reason you'd want to throw an `ArgumentException` (or derived) and that is if the method can tell *for sure* that the received `idOfUser` is out of range (negative, zero... depending on your db specs). You'd then want to throw an `ArgumentOutOfRange` exception. Other than that, favor a design that produces a custom operation result object indicating success or failure (and the reason for the latter). — Crono, Oct 25 '18 at 18:49
`ArgumentException` and derived classes are `developer` errors and there should **never** be a reason to handle them at runtime. **DON'T EVER** design an API in a way that forces the using dev to rely on handling an `ArgumentException` to deal with the possibility of missing data. This is *not* what those exception exists for. — Crono, Oct 25 '18 at 18:51

score 8 · Answer 1 · edited May 23 '17 at 12:22

8

The first

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

should be

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

If possible you shouldn't throw ArgumentException directly

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

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

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

It is defined in System.Data: System.Data.ObjectNotFoundException.

edited May 23 '17 at 12:22

Community

1
1

answered Jun 22 '15 at 13:23

xanatos

109,618
12
197
280

It is in the System.Data namespace but requires adding the Entity dll to the project. – Rhs Jun 23 '15 at 20:57
I have a validation method with parameters (DateTime? minDate, DateTime? maxDate, bool includeMinDate, bool includeMaxDate). If the caller specifies both a minDate and a maxDate, then it throws an ArgumentException if minDate is greater than maxDate, and it also throws an ArgumentException if minDate is equal to maxDate and includeMinDate and includeMax date are not both true (i.e. it is not valid to have the lower bound inclusive while the upper bound is exclusive (or vice versa) when both the upper and lower bounds are equal). So an ArgumentException is appropriate for combinations as well. – Triynko Jun 14 '18 at 19:06

score 1 · Accepted Answer · answered Jun 22 '15 at 14:15

As the name suggests, an ArgumentException is an exception about an argument. It means the argument was somehow inherently wrong.

The general form is:

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

If the only possible way that GetUserById could fail was that there was something inherently incorrect with the value of idOfUser then the following would both 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 it turned out to be for some reason faster or less wasteful of some resource to test user after the fact than idOfUser before the fact and if there were no side-effects of calling GetUserById, and if the difference actually mattered then maybe the second version would be a reasonable optimisation of the first.

But that only holds if all of the ifs above hold, and it's then a weird way of detecting an invalid argument that has some specific advantage where we benefit from the encapsulation of methods by hiding that weirdness from everything else.

Chances are there could be a valid idOfUser for which there was no corresponding user, in which case it certainly wasn't an argument exception.

Hi Jon. Appreciate this an old one, but is it worth fixing the user of `ArgumentException` in your code snippet? In the constructor shown, the string is used to specify a message, not the name of the argument (https://learn.microsoft.com/en-us/dotnet/api/system.argumentexception.-ctor?view=net-6.0#System_ArgumentException__ctor_System_String_). — Dave Watts, Dec 14 '21 at 10:53

Correct Usage of ArgumentException?

2 Answers2