Best way to handle null properties on a method parameter

Question

When checking a method's parameter, I throw an ArgumentNullException if it's null. See the first line in the method below. But what about properties on the parameter that shouldn't be null? If I try to handle them the same way, I get a code analysis error:

CA2208 Instantiate argument exceptions correctly Method 'PriorityDeratingComponentLogic.CreateItem(IvSimulation)' passes 'ivSimulation.SolarPanel' as the 'paramName' argument to a 'ArgumentNullException' constructor. Replace this argument with one of the method's parameter names. Note that the provided parameter name should have the exact casing as declared on the method.

public DeratingComponentBase CreateItem(IvSimulation ivSimulation)
{
    if (ivSimulation == null) { throw new ArgumentNullException("ivSimulation"); }
    if (ivSimulation.SolarPanel == null) { throw new ArgumentNullException("ivSimulation.SolarPanel"); }
    if (ivSimulation.GlobalEquipment == null) { throw new ArgumentNullException("ivSimulation.GlobalEquipment"); }

    // ... method body here
}

Is the CA error something I should suppress, or is there a generally-accepted way to better handle this? Perhaps the issue is upstream and we shouldn't even have to check for those properties being null at this point?

Yeah, that code analysis error message looks stupid. This looks perfectly fine to me. Stuff like this is why I've never used code analysis software before... — Jashaszun, Jul 22 '15 at 16:31
`IvSimulation` should prevent construction of invalid values, so it should validate `SolarPanel` and `GlobalEquipment`. — Lee, Jul 22 '15 at 16:33

Sergey Kalinichenko · Accepted Answer · 2015-07-22T16:39:14.620

10

Throwing an ArgumentNullException is intended to indicate that the argument itself is null. However, when one of argument components is null, but the argument itself isn't, the proper exception is ArgumentException with the name of the argument as parameter:

if (ivSimulation.GlobalEquipment == null) {
    throw new ArgumentException("GlobalEquipment cannot be null", "ivSimulation");
 }

This provides the information about the error in the message, while specifying ivSimulation as the parameter name.

Note: I assume that you have no choice of validating GlobalEquipment in the constructor of IvSimulation, because throwing ArgumentNullException would be a perfectly valid choice in that constructor.

edited Jul 22 '15 at 16:39

answered Jul 22 '15 at 16:35

Sergey Kalinichenko

714,442
84
1,110
1,523

1

You could do something like this: `throw new ArgumentException("Global Equipment cannot be null", "ivSimulation");` to give more information. – Eric Hotinger Jul 22 '15 at 16:36
@EricHotinger That's a better choice indeed, thank you very much! – Sergey Kalinichenko Jul 22 '15 at 16:39
I'd prefer a **NullReferenceException** ("The exception that is thrown when there is an attempt to dereference a null object reference.") instead of an ArgumentException ("The exception that is thrown when one of the arguments provided to a method is not valid.") because the argument itself is valid, but it's null. – Jan Köhler Jul 22 '15 at 16:46

score 1 · Answer 2 · answered Jul 22 '15 at 16:37

It can be argued that a null property on a non-null parameter variable isn't really a null argument, so the ArgumentNullException is not actually appropriate in this case. (Clearly, others may disagree, which is fine.) This feels to me like a qualitatively different situation. The argument isn't missing, but it is in an invalid state. I think I would use an InvalidOperationException for cases like this, where an object is in an invalid state.

Best way to handle null properties on a method parameter

2 Answers2