Thursday, 28 August 2014

Optional parameters considered harmful (in public libraries)


UPDATE: I was being stupid. See here for the right way to do this

TAKE THIS POST WITH A HUGE PINCH OF SALT

Optional parameters are great; they can really clean down the number of overloads needed on some APIs where the intent can be very different in different scenarios. But; they can hurt. Consider the following:
static void Compute(int value, double factor = 1.0,
    string caption = null)
{ /* ... */ }

Great; our callers can use Compute(123), or Compute(123, 2.0, "awesome"). All is well in the world. Then a few months later, you realize that you need more options. The nice thing is, you can just add them at the end, so our method becomes:
static void Compute(int value, double factor = 1.0,
  string caption = null, Uri target = null)
{ /* ... */ }

This works great if you are recompiling everything that uses this method, but it isn’t so great if you are a library author; the method could be used inside another assembly that you don’t control and can’t force to be recompiled. If someone updates your library without rebuilding that other dll, it will start throwing MissingMethodException; not great.

OK, you think; I’ll just add it as an overload instead:
static void Compute(int value, double factor = 1.0,
    string caption = null)
{ Compute(value, factor, caption, null); }
static void Compute(int value, double factor = 1.0,
    string caption = null, Uri target = null)
{ /* ... */ }

And you test Compute(123, 1.0, "abc") and Compute(123, target: foo), and everything works fine; great! ship it! No, don’t. You see, what doesn’t work is: Compute(123). This instead creates a compiler error:


The call is ambiguous between the following methods or properties: 'blah.Compute(int, double, string)' and 'blah.Compute(int, double, string, System.Uri)'

Well, damn…

This is by no means new or different; this has been there for quite a while now – but it still sometimes trips me (and other people) up. It would be really nice if the compiler would have a heuristic for this scenario such as “pick the one with the fewest parameters”, but; not today. A limitation to be wary of (if you are a library author). For reference, it bit me hard when I failed to add CancellationToken as a parameter on some *Async methods in Dapper that had optional parameters - and of course, I then couldn't add it after the fact.

9 comments:

James Curran said...

The problem with you last example is that the second overload should be defined as:


static void Compute(int value, double factor,
string caption, Uri target)


You gain no utility at all by defaulting those parameters here (provided that other overload exists)

Marc Gravell said...

@James yes you do:

Foo(123, target: foo);

(taken directly from the post) - without having to specify all the other defaults.

Patrick Huizinga said...

Actually James is on to something. :)

Change the first overload to:

static void Compute(int value, double factor, string caption, Uri target) // look mommy, no defaults!

And then let only the new method have all the defaults.

From a binary perspective the first method hasn't changed, so older compiles continue to work. But the compiler will now always pick the new method (unless there is an exact match for the first), so new compiles will also work.

Patrick Huizinga said...

oops, I mean

static void Compute(int value, double factor, string caption) // look mommy, no defaults!

as the new first method, without the target param.

Marc Gravell said...

That may indeed solve the problem. Will try. Good thinking.

abatishchev said...

I hardly can imagine a scenario of updating dependency without recompiling.
Usually it's a nuget package. Ok, it's not, then even more hardly.
How to do it? Put new dlls in bin folder on production web server?

Marc Gravell said...

@abatishchev that isn't the only problem; dapper has at least 47 nuget packages that depend *on it* - and that isn't including local MyCompanyDataAccess etc - so breaking compatibility is a huge no-no.

Sean Garrett said...

I think optional parameters are sslightly misleading in hoe they're perceived to work. They don't cause the compiler to magically create overloads for you. What they do is cause the compiler to insert constant values in the method call for the missing parameters.
This means that the CALLING class is where the default value lives post compile not the one that specified the value.

Marc Gravell said...

@Patrick thanks for keeping me honest: http://blog.marcgravell.com/2014/09/optional-parameters-maybe-not-so-harmful.html