Coding Style Tip: Use Named Arguments When Passing null to Methods

I noticed this pattern in play while working on the System.Text.Json code in .NET Core (corefx) and immediately fell in love with it. Consider this method signature:

        internal static JsonPropertyInfo CreateProperty(
            Type declaredPropertyType,
            Type runtimePropertyType,
            Type implementedPropertyType,
            PropertyInfo propertyInfo,
            Type parentClassType,
            JsonConverter converter,
            JsonSerializerOptions options)

Lots of complex stuff being passed into this method and much of it is optional. That’s kind of a “code smell” right? This method is a heavy-lifter or swiss-army-knife type. Meaning, it’s probably doing too much and needs to be refactored. But it’s internal to the assembly, not part of the public API, and does the job it needs to do and nothing more. We all have some code like this. Maybe we could refactor it into a bunch of builders and factories, but is it worth it? I try to make that call based on whether or not we’ll need to make changes in this code again. If this code is likely to change a lot, invest some time. If it will never change again, it’s probably well enough. Don’t skip out on your unit tests though, they will help you if you ever want to refactor it in the future.

But this method isn’t the topic, just its signature. How we are going to call this method is where things get interesting. Notice there are no optional arguments ([type] name = defaultValue).

Let’s imagine some developer comes along and opens a PR calling our helper method like this:

            return CreateProperty(
                declaredPropertyType,
                runtimePropertyType,
                null,
                null,
                null,
                null,
                null);

There’s nothing wrong with this code. But it is really hard to read! You just looked at the method signature, do you remember all the parameters without going and looking them up? Probably not.

Does this seem better?

            return CreateProperty(
                declaredPropertyType,
                runtimePropertyType,
                implementedPropertyType: null,
                propertyInfo: null,
                parentClassType: null,
                converter: null,
                options: null);

Undeniably, right? C# actually allows us to use parameter names whenever we want. You can also specify things out of order, but be careful with that… it could make code harder to read and therefore less maintainable.

For me, specifying the parameter name when you are passing null is a homerun for anything non-trivial. If your method is public string ConvertObjectToStringOrThrowIfNull(object instance) you got me, you probably don’t need it in this case because code is obvious enough at a glance with just “null” being passed in. It’s a guideline, not a rule, OK?

You could take this a step further, too. Here’s another call site:

            return CreateProperty(
                Type,
                Type,
                Type,
                propertyInfo: null,
                Type,
                converter: null,
                options);

In this case “options” is obvious, but Type is used for a lot of the parameters. If you want to know what “Type” represents in a given position, you are going to need to look it up. If we change our rule to “use named arguments when variable name does not obviously convey usage” it might look like this:

            return CreateProperty(
                declaredPropertyType: Type,
                runtimePropertyType: Type,
                implementedPropertyType: Type,
                propertyInfo: null,
                parentClassType: Type,
                converter: null,
                options);

Does that achieve maximum readability? Go forth and debate with your teams!