Design Guideline Update: handling nulls in public APIs


We had a recent thread internally that resulted in me updating the guidelines below… Please let me know if you have any questions or comments.

Do provide overloads for methods with optional arguments.  If a method takes some arguments that are not required provide overloads of the method without the optional arguments.  See section X.Y on method overloading.  That is if you have this method here geometry is optional, provide an overload without it as well.
public void DrawGeometry(Brush brush, Pen pen, Geometry geometry);
public void DrawGeometry(Brush brush, Pen pen);

  Do allow null to be passed for optional arguments.  If a method takes optional arguments that are reference types, allow null to be passed to indicate the default value should be used.  This avoids the problem of having to check for null before calling an API as shown below:
      if (brush == null) DrawGeometry(pen, geometry);
      else DrawGeometry(pen, geometry, brush);

Annotation (Adam Smith): It sure seems that we’re providing a less startling API if we define reasonable, expected behavior when “null” is passed for a param.  If that’s really impossible, sure, throw an exception, but to throw when there’s a consistent, non-crashing thing we could have done seems unhelpful. 

 

Comments (12)

  1. mikeb says:

    The example code doesn’t fit the guideline: ‘brush’ is not an optional parameter (at least if the example is supposed to match with the method signatures provided previously).
    <br>
    <br>The example should look like:
    <br>
    <br> if (geometry == null) DrawGeometry( brush, pen);
    <br> else DrawGeometry( brush, pen, geometry);
    <br>

  2. I think that these new rules reduce the readability of the code. If I can now write:

    Draw(brush, pen, geometry)

    and have it mean the same thing as:

    Draw(brush)

    if pen and geometry are null, that means I have to keep more state in my head to understand the code. I also have to remember that a null means the same thing as not using the parameter at all – at least in some cases. In other cases, it might not mean the same thing.

    I don’t think the code where you check for null and then call is very common, so I wouldn’t make this change.

    I also don’t understand the second code block, as it shows a method defined as:

    public void DrawGeometry(Brush brush, Pen pen, Geometry geometry);

    public void DrawGeometry(Pen pen, Geometry geometry);

    which I think would be a pretty bad way to order parameters.

  3. Sorry, that was just a type-o on my part… I fixed it above… optionals should aways be at the end.

    if (brush == null) DrawGeometry(pen, geometry);

    else DrawGeometry(pen, geometry, brush);

  4. I think the design guideline is great. Its exactly how I currently handle overloaded methods and allowable null values for parameters.

  5. AT says:

    I dislike this.
    <br>
    <br>null values are evil.
    <br>In big project there can be a lot of situations then value instead of been correct one – magicaly changed / initialised with null reference.
    <br>
    <br>With current suggestion this will result program still working – but not doing that it expected to do and cause unpredicable results.
    <br>
    <br>In some scenarions – I preffer program to crash in this situation. But I also agree – for less reliable and less critial software this can be good to do best possible action without throwing exception.
    <br>
    <br>I’ve provided an overload without requerements to supply this argument – why do you think I’ve spend my time to do this ? Just to make your code pretty ? Or to make your code error prone ?
    <br>I do not allow null reference params in publicily exposed API.
    <br>If user call method with 3 arguments – it’s expected to do exactly that I’ve wrote in documentation – use all arguments to perform action. If you does not suply one – this is your fault.
    <br>
    <br>The best that I can offer – is something like Brush.Default. Thus third-party (not yours !!) code can looks like
    <br>DrawGeometry(pen, geometry, brush!=null ? brush : Brush.Default);
    <br>
    <br>This will allow to use more that one default argument and will definitely prevent all errors from using non-initialised values.
    <br>
    <br>Using additional .Default values seems like a duplication of null value – but this is not true. Using this additional value you add one more state to variable:
    <br>Unknown value, _Default value_, Specific value.

  6. orangy says:

    First, I’d make example code more consistent. At the top of the post you have geometry optional, while at the bottom you have brush optional. I think that method DrawGeometry should not have geometry optional 🙂 So that should be DrawGeometry(geometry, pen, brush), because later you could make pen optional as well.

    Second, it is lack of default value for parameter in C# that makes such "design" guidelines. Having default value would address most issues discussed.

  7. David Levine says:

    I agree with the guideline though I sympathize with Eric’s reason. I think the real problem is with the fundamental nature of optional arguments – the only way to avoid the issue is to not use optional arguments at all. There is no mechanism or syntax in C# for indicating the difference between an optional argument versus a required argument. IOW, if it is required and the arg is null then it ought to throw an exception; if it is optional then a null value is legal. It may be confusing but it does leave it up to the class designer to make that decision.

    I’d add guidelines that dictate that arguments should be in the same order in the arg list; i.e. a guideline should dictate that your typo was a violation of the guideline. I know this seems obvious but it ought to be formalized. IOW, these signatures would violate the guidelines…

    Draw(Brush);

    Draw(Brush,Pen);

    Draw(Brush,Geometry,Pen);

    It should be…

    Draw(Brush);

    Draw(Brush,Pen);

    Draw(Brush,Pen,Geometry);

  8. I just quit using method overloads for optional parameters. It’s good to have the rule than an optional param is assumed to be null, but when you add in value types, it’s that many more things to remember. I guess the general rule is that assume the optional param is assigned that type’s default value, but it’s just one more thing to think to think about during coding. Ultimately, I don’t mind having to plug in a bunch of nulls and 0s in a method param if it means I have that fewer to sift through during AutoComplete.

  9. Keith Hill says:

    Hmm, I don’t like this new guideline as I think that it will result in some subtle bugs. If I use an API that accepts null as optional parameters then it might work in a way I didn’t intend when unbeknownst to me I get a null given to me, to be used as a parameter to the API. That puts the onus on me now to validate parameters before calling the API:

    Geometry geometry = Foo.Geometry;

    if (geometry == null) throw new ??Exception("geometry");

    DrawSomething(brush, pen, geometry);

    So what exception would I throw above? If this check happened in DrawSomething then it is clear it should throw an ArgumentNullException.

    Personally I think conflating null and the notion of "default" parameters is a bad idea when you consider that the presence of "null" far too often means a programmer did something wrong.

  10. why allow null at all?

    When you can do this:

    void DrawGeometry(pen, geometry)

    {

    DrawGeometry(pen, geometry, new brush(Pen.Black);

    }

    Have I missed something?

    Best, Johan

  11. Then if I call the longer DrawGeometry overload and happen to pass null (a variable was null) then I’d get an exception… that means I’d have to code around that which isn’t pretty

  12. Keith Hill says:

    Yes, but if I went out of my way to call the longer DrawGeometry overload I probably expected to be using a valid brush (or whatever the parameter type is). By silently "handling" the null you may have just made this issue much harder for me to debug. Tip 32 from The Pragmatic Programmer is "Crash Early". Systems that do too much for you might be nice for smaller, one-off programers but can really suck for large programs.