Optional Parameters, Good or Bad?

Question

I am writing and browsing through a lot of methods in the project im working with and as much as I think overloads are useful I think that having a simple optional parameter with a default value can get around the problem aiding in writing more readable and I would think efficient code.

Now I hear that using these parmeters in the methods could carry nasty side effects.

What are these side effects and is it worth the risk of using these parameters to keep the code clean ???

Answer 1

I'll start by prefacing my answer by saying Any language feature can be used well or it can be used poorly. Optional parameters have some drawbacks, just like declaring locals as var does, or generics.

What are these side effects

Two come to mind.

The first being that the default value for optional parameters are compile time constants that are embedded in the consumer of the method. Let's say I have this class in AssemblyA:

public class Foo
{
    public void Bar(string baz = "cat")
    {
        //Omitted
    }
}

And this in AssemblyB:

public void CallBar()
{
    new Foo().Bar();
}

What really ends up being produced is this, in assemblyB:

public void CallBar()
{
    new Foo().Bar("cat");
}

So, if you were to ever change your default value on Bar, both assemblyA and assemblyB would need to be recompiled. Because of this, I tend not to declare methods as public if they use optional parameters, rather internal or private. If I needed to declare it as public, I would use overloads.

The second issue being how they interact with interfaces and polymorphism. Take this interface:

public interface IBar
{
     void Foo(string baz = "cat");
}

and this class:

public class Bar : IBar
{
     public void Foo(string baz = "dog")
     {
         Console.WriteLine(baz);
     }
}

These lines will print different things:

IBar bar1 = new Bar();
bar1.Foo(); //Prints "cat"
var bar2 = new Bar();
bar2.Foo(); //Prints "dog"

Those are two negatives that come to mind. However, there are positives, as well. Consider this method:

void Foo(string bar = "bar", string baz = "baz", string yat = "yat")
{
}

Creating methods that offer all the possible permutations as default would be several if not dozens of lines of code.

Conclusion: optional parameters are good, and they can be bad. Just like anything else.

Answer 2

Necromancing.
The thing with optional parameters is, they are BAD because they are unintuitive - meaning they do NOT behave the way you would expect it.

Here's why:
They break ABI compatibility !
(and strictly speaking, they also break API-compatiblity, when used in constructors)

For example:

You have a DLL, in which you have code such as this

public void Foo(string a = "dog", string b = "cat", string c = "mouse")
{
    Console.WriteLine(a);
    Console.WriteLine(b);
    Console.WriteLine(c);
}

Now what kinda happens is, you expect the compiler to generate this code behind the scenes:

public void Foo(string a, string b, string c)
{
    Console.WriteLine(a);
    Console.WriteLine(b);
    Console.WriteLine(c);
}

public void Foo(string a, string b)
{
    Foo(a, b, "mouse");        
}

public void Foo(string a)
{
    Foo(a, "cat", "mouse");
}

public void Foo()
{
    Foo("dog", "cat", "mouse");
}

or perhaps more realistically, you would expect it to pass NULLs and do

public void Foo(string a, string b, string c)
{
    if(a == null) a = "dog";
    if(b == null) b = "cat";
    if(c == null) c = "mouse";

    Console.WriteLine(a);
    Console.WriteLine(b);
    Console.WriteLine(c);
}

so you can change the default-arguments at one place.

But this is not what the C# compiler does, because then you couldn't do:

Foo(a:"dog", c:"dogfood");

So instead the C# compiler does this:

Everywhere where you write e.g.

Foo(a:"dog", c:"mouse");
or Foo(a:"dog");
or Foo(a:"dog", b:"bla");

It substitutes it with

Foo(your_value_for_a_or_default, your_value_for_b_or_default, your_value_for_c_or_default);

So that means if you add another default-value, change a default-value, remove a value, you don't break API-compatiblity, but you break ABI-compatibility.

So what this means is, if you just replace the DLL out of all files that compose an application, you'll break every application out there that uses your DLL. That's rather bad. Because if your DLL contains a bad bug, and I have to replace it, I have to recompile my entire application with your latest DLL. That might contain a lot of changes, so I can't do it quickly. I also might not have the old source code handy, and the application might be in a major modification, with no idea what commit the old version of the application was compiled on. So I might not be able to recompile at this time. That is very bad.

And as for only using it in PUBLIC methods, not private, protected or internal.
Yea, nice try, but one can still use private, protected or internal methods with reflection. Not because one wants to, but because it sometimes is necessary, as there is no other way. (Example).

Interfaces have already been mentioned by vcsjones.
The problem there is code-duplication (which allows for divergent default-values - or ignoring of default-values).

But the real bummer is, that in addition to that, you can now introduce API-breaking-changes in Constructors...
Example:

public class SomeClass
{
    public SomeClass(bool aTinyLittleBitOfSomethingNew = true)
    {
    }
}

And now, everywhere where you use

System.Activator.CreateInstance<SomeClass>();

you'll now get a RUNTIME exception, because now there is NO parameter-less constructor...
The compiler won't be able to catch this at compile time.
Good night if you happen to have a lot of Activator.CreateInstances in your code.
You'll be screwed, and screwed badly.
Bonus points will be awarded if some of the code you have to maintain uses reflection to create class instances, or use reflection to access private/protected/internal methods...

Don't use optional parameters !

Especially not in class constructors.
(Disclaimer: sometimes, there simply is no other way - e.g. an attribute on a property that takes the name of the property as constructor argument automagically - but try to limit it to these few cases, especially if you can make due with overloading)


I guess theoretically they are fine for quick prototyping, but only for that.
But since prototypes have a strong tendency to go productive (at least in the company I currently work), don't use it for that, either.