AltF4_
AltF4_

Reputation: 2470

Optional Parameters, Good or Bad?

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 ???

Upvotes: 22

Views: 18330

Answers (5)

Stefan Steiger
Stefan Steiger

Reputation: 82176

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.

Upvotes: 16

HankTheTank
HankTheTank

Reputation: 585

Often I see optional parameters in C# like IMyInterface parameter = null. Especially when I see that in constructors I would even say it'S a code smell. I know that's a hard verdict - but in this case it obscures your dependencies, which is bad.

Like vcsjones said, you can use those language features right, but I believe optional parameters should be used only in some edge-cases.

my opinion.

Upvotes: -1

anaximander
anaximander

Reputation: 7140

I'd say that it depends how different the method becomes when you include or omit that parameter.

If a method's behaviour and internal functioning is very different without a parameter, then make it an overload. If you're using optional parameters to change behaviour, DON'T. Instead of having a method that does one thing with one parameter, and something different when you pass in a second one, have one method that does one thing, and a different method that does the other thing. If their behaviour differs greatly, then they should probably be entirely separate, and not overloads with the same name.

If you need to know whether a parameter was user-specified or left blank, then consider making it an overload. Sometimes you can use nullable values if the place they're being passed in from won't allow nulls, but generally you can't rule out the possibility that the user passed null, so if you need to know where the value came from as well as what the value is, don't use optional parameters.

Above all, remember that the optional parameters should (kinda by definition) be used for things that have a small, trivial or otherwise unimportant effect on the outcome of the method. If you change the default value, any place that calls the method without specifying a value should still be happy with the result. If you change the default and then find that some other bit of code that calls the method with the optional parameter left blank is now not working how it should, then it probably shouldn't have been an optional parameter.

Places where it can be a good idea to use optional parameters are:

  • Methods where it's safe to just set something to a default if a value isn't provided. This basically covers anything where the caller might not know or care what the value is. A good example is in encryption methods - the caller may just think "I don't know crypto, I don't know what value R should be set to, I just want this to be encrypted", in which case you set the defaults to sensible values. Often these start out as a method with an internal variable that you then move to be user-provided. It's pointless making two methods when the only difference is that one has var foo = bar; somewhere at the start.
  • Methods that have a set of parameters, but not all of them are needed. This is quite common with constructors; you'll see overloads that each set different combinations of the various properties, but if there's three or four parameters that may or may not need to be set, that can require a lot of overloads to cover all the possible combinations (it's basically a handshake problem), and all these overloads have more or less identical behaviour internally. You can solve this by having most of them just set defaults and call the one that sets all parameters, but it's less code to use optional parameters.
  • Methods where the coder calling them might want to set parameters, but you want them to know what a "normal" value is. For example, the encryption method we mentioned earlier might require various parameters for whatever maths goes on internally. A coder might see that they can pass in values for workFactor or blockSize, but they may not know what "normal" values are for these. Commenting and documentation will help here, but so will optional parameters - the coder will see in the signature [workFactor = 24], [blockSize = 256] which helps them judge what kind of values are sensible. (Of course, this is no excuse to not comment and document your code properly.)

Upvotes: 4

vcsjones
vcsjones

Reputation: 141618

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.

Upvotes: 25

Geeky Guy
Geeky Guy

Reputation: 9399

You're not making more readable and efficient code.

First, your method signatures will be gratuitously longer.

Second, overloads don't exist for the sole purpose of using default values - a quick look at the Convert class should show you that. Many times overloaded methods have different execution paths, which will become spaghetti code in your single non overloaded method.

Third, sometimes you need to know whether a value was used as input. How would you then know whether the user passed those values, if he happens to use the same value as the default one you were using?

Upvotes: 3

Related Questions