20

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

Guru Stron
  • 102,774
  • 10
  • 95
  • 132
AltF4_
  • 2,312
  • 5
  • 36
  • 56
  • 2
    Possible duplicate of http://stackoverflow.com/questions/251868/should-you-declare-methods-using-overloads-or-optional-parameters-in-c-sharp-4-0?rq=1 – Kenneth May 15 '13 at 13:35
  • @Kenneth you seem to be right ... The answers on the other questions are also useful. – AltF4_ May 15 '13 at 13:38
  • 3
    We can't just answer "good" or "bad". There are good usages, and of course, bad usages / abuses. – ken2k May 15 '13 at 13:41
  • 1
    possible duplicate of [method overloading vs optional parameter in C# 4.0](http://stackoverflow.com/questions/3316402/method-overloading-vs-optional-parameter-in-c-sharp-4-0) – Steve B May 15 '13 at 13:56

5 Answers5

23

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.

vcsjones
  • 138,677
  • 31
  • 291
  • 286
16

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.
Stefan Steiger
  • 78,642
  • 66
  • 377
  • 442
5

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.)
anaximander
  • 7,083
  • 3
  • 44
  • 62
3

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?

Geeky Guy
  • 9,229
  • 4
  • 42
  • 62
  • 1
    Your point seems to be that there are a lot of cases where using optional parameters wouldn't make sense. I'll agree with that. On the other hand, if the only purpose of a method overload which takes a small number of parameters is to call an overload which takes more while passing for the extra parameters default values which aren't apt to change (e.g. `false`, 0, `null`, etc.), simply specifying the default parameters may make things more efficient, concise, and readable, than having extra overloads. – supercat Aug 18 '13 at 00:21
  • 1
    I agree that your code will be more efficient, if you are pedantic enough - you'll end up saving a couple miliseconds a day in code execution, and your execution stack would be one level shallower. But having spent years doing code reviews for my peers, I cabsolutely disagree that it would make your code more readable. I highly recommend you a book called [Clean Code](http://www.amazon.com/Clean-Code-Handbook-Craftsmanship-ebook/dp/B001GSTOAM/ref=sr_1_1?s=digital-text&ie=UTF8&qid=1376795618&sr=1-1&keywords=clean+code). – Geeky Guy Aug 18 '13 at 03:15
  • I favor writing code which expresses what one is *actually trying to do*. If one's goal is to have default values for some parameters, the fact that a method has optional parameters with those default values would in and of itself express precisely that. Overloads with varying numbers of parameters would work, but one would have to actually examine the code of each overload to ascertain whether it might serve some other purpose. Further, interface methods can have optional parameters without emburdening implementations. By contrast, every overload requires extra code in every implementation. – supercat Aug 18 '13 at 17:49
  • BTW, with regard to the latter issue, what I'd really like to see would be a mechanism for interfaces to be associated with static classes, which could then include members or overloads which would be treated as part of the interface (somewhat like extension methods, but with somewhat better scoping rules). It would be especially nice if the run-time class loader could attempt to fill in "missing" interface methods by searching an associated static class for suitable methods and--if they exist--auto-generating method implementations to call them. That could save a lot of boilerplate. – supercat Aug 18 '13 at 18:04
0

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.

HankTheTank
  • 537
  • 5
  • 15