9

Suppose I have a class Foo with a complex property Bar. Then, suppose I have a method like the following in some other class:

public void DoSomething(Foo foo)
{
    if (foo == null)
        throw new ArgumentNullException("foo");
    if (foo.Bar == null)
        throw new ArgumentNullException("bar");
}

Is the use of an ArgumentNullException appropriate here even though, strictly speaking, foo.Bar is not an argument in this case? I have read and can appreciate that it is not appropriate to throw a NullReferenceException manually. Is this telling me that I need to abstract?

public void DoSomething(Foo foo)
{
    if (foo == null)
        throw new ArgumentNullException("foo");
    DoSomethingElse(foo.Bar);
}

private void DoSomethingElse(Bar bar)
{
    if (bar == null)
        throw new ArgumentNullException("bar");
}

Is my first code snippet the "correct" usage of ArgumentNullException? What is the conventional way of handling this situation?

Thanks.

Community
  • 1
  • 1
Ant P
  • 24,820
  • 5
  • 68
  • 105
  • 2
    You can throw `new ArgumentException("foo")` when it is not valid instance of `Foo` (e.g. one of it's properties is `null`) – MarcinJuraszek Feb 26 '13 at 21:09
  • 2
    Be aware when you are throwing an `ArgumentException` that the single string constructor expects a message [`new ArgumentException(string message)`](http://msdn.microsoft.com/en-us/library/wtxc6334.aspx). If you are going to specify the parameter name use [`new ArgumentException(string message, string paramName)`](http://msdn.microsoft.com/en-us/library/sxykka64.aspx). – JG in SD Feb 26 '13 at 21:16

1 Answers1

15

Ideally, the Foo class would ensure that its Bar property is never null. If that's not possible, I would throw an ArgumentException in this case since the argument is not null, but it is invalid.

Lee
  • 142,018
  • 20
  • 234
  • 287
  • 1
    Supposing that it's valid for `Foo.Bar` to be null, but `DoSomething` in particular needs `foo.Bar` to have a value, would you suggest that `DoSomething` should always handle this with a default execution path (returning a default value, if applicable) that does not throw an exception at all? – Ant P Feb 26 '13 at 21:24
  • @AntP - That depends on what `DoSomething` does. If it's valid to return a default value then that might be preferable, but I would throw an exception rather than return some 'special' return value like null or -1. – Lee Feb 26 '13 at 21:34