Default argument value vs. overload

Development
1

Recently the following was added to the VTK coding conventions document:

  1. Prefer overloading functions to default arguments.
    Rationale: Default function arguments in C++ are a tempting way to add an argument to a function while maintaining easy backwards compatibility. However, if you later want to add another argument to the list in a way that preserves backwards compatibility, it, too, must be a default argument. To supply the second of these arguments in a call forces you to also supply the first argument, even if it is the default value. As a result, this is not a clean way to add a argument to a function. Instead, function overloading should be preferred.

Iā€™ve asked for an explanation of this rule in the merge request but it was not really answered there - no example or explanation was given. The question has also come up in this topic, so I though I would bring up this question here: How overloading is a cleaner way to add arguments to a function?

How is it even feasible to pass multiple optional arguments via overloading? Adding overloaded methods for multiple optional arguments works if you are so lucky that the all the optional arguments have incompatible types - but most of the time this is not the case. For example, if you have 2 optional float arguments, then you can specify a method with default arguments:

void DoSomething(double a, double b=-1, double c=-1);

But how would you implement that with overloaded methods?

void DoSomething(double a);
void DoSomething(double a, double b);
void DoSomething(double a, double c); // this would throw a build error, because it is equivalent with the previous line
void DoSomething(double a, double b, double c);

There are some valid reasons for preferring overload in some cases and default value in others - see for example the discussion in Google C++ style guide, but they are not related to managing backward compatibility more cleanly.

Could we replace this guidance with something more balanced, and better explained, such as Googleā€™s? If nobody wants to spend time with better explaining the current recommendation then thatā€™s completely fine too, it means that it is actually not that important; but in that case the best is to simply remove the recommendation to keep things simpler and avoid unnecessary discussions. I would just want to avoid having some questionable rules in the VTK coding conventions, because that could lead to developers not taking the entire document seriously.

2

Sorry, Ben replied and there was no followup message that his reply was not satisfactory, so I let it stand. The guidance stems from a code review and discussion in this merge request.

For a concrete example, letā€™s say you want to add optional behavior to a function

void DoSomething(double a);

You could do this with an additional parameter with a default argument:

void DoSomething(double a, bool useNewMode=false);

That preserves backwards compatibility because now all calls that were simply

DoSomething(2.0);

effectively become

DoSomething(2.0, false);

In my opinion a way to ā€œsneakā€ in a new parameter to a function signature. For one parameter itā€™s not so bad.

Now some time passes in and another developer wants to add another parameter this same way to this same function:

void DoSomething(double a, bool useNewMode=false, double multiplier=1.0);

Great, we have preserved backwards compatibility once again. Now, however, if you only care about changing the new parameter multiplier, you also need to specify the previously added useNewMode, even if the default mode is fine. So now you have to look up what the default value is and your call where you want to provide a specific multiplier value now must be

DoSomething(double a, false, 1.5);

If you have used Qt, you are no doubt familiar with the prevalence of default arguments in that library. You face the same issue if you want to provide a non-default argument at the end of a long argument list - now every argument needs to be provided, not just the thing you care about changing. Qt has mostly done this in a smart way by putting the arguments most often changed early in function argument lists, but they have benefited from being able to design their API from the beginning.

In a toolkit like VTK that grows organically, the tendency to grow parameters with default arguments this way has come up in code reviews time and time again. Developers want to add ā€œjust one more thingā€ and default arguments seem like a clean way to do it in all cases.

However, in my opinion this is not a good way to do things. Instead, I prefer providing overloads to specify subsets of arguments, e.g.

void DoSomething(double a, bool useNewMode, double multiplier);
void DoSomething(double a, bool useNewMode)
{
  DoSomething(a, useNewMode, 1.0);
}
void DoSomething(double a, double multiplier)
{
  DoSomething(a, false, multiplier);
}
void DoSomething(double a)
{
  DoSomething(a, false, 1.0);
}

With this approach, backwards compatibility is easily preserved, and you need only define function signatures you care about when providing new functionality. In turn, callers to those functions donā€™t need to specify the full laundry list of arguments.

Does this approach run into potential combinatoric explosion? Sure. But I hope that the approach of providing overloads might encourage rethinking how to add a new feature more than simply tacking on another parameter with a default argument would.

Note that the VTK coding conventions say ā€œpreferredā€, not ā€œrequiredā€. In the example you cite, you indeed cannot use overloading to provide the two different versions of void DoSomething(double, double) you cite, and default arguments may be acceptable in this case. Note that if you only want to provide a and c to the signature with default arguments, you also need to provide b=-1 in your calls, which is annoying :slight_smile:

The discussion is about whether to use default arguments or overloading when adding arguments to existing function calls while preserving backwards compatibility - either approach is ā€œcleanā€ in the sense that you donā€™t have to make a bunch of changes in calling code. The convention suggests which option to prefer.

I hope Iā€™ve answered your questions as to the rationale. If you have suggestions for how to clarify the preference expressed in the VTK coding conventions, Iā€™m happy to see them improved.

4 Likes
3

So now you have to look up what the default value is and your call where you want to provide a specific multiplier value now must be

DoSomething(double a, false, 1.5);

Also, if at some point we want to deprecate the second argument (useNewMode), all of the calls using explicitly the default value needs to be updated, so that adds some extra work (even if the fix is trivial)

1 Like
4

Note that overloads should be chosen with care, because itā€™s easy to create overloads that break existing code. For example:

void DoSomething(double a, bool useNewMode);
void DoSomething(double a, double multiplier);

void myfunction()
{
  // this used to work, until someone overloaded the function
  DoSomething(10, 1);
}

Example compile error:

ambiguous.cxx:7:14: error: call of overloaded ā€˜DoSomething(int, int)ā€™ is ambiguous
   17 |   DoSomething(10, 1);
      |   ~~~~~~~~~~~^~~~~~~
ambiguous.cxx:1:6: note: candidate: ā€˜void DoSomething(double, bool)ā€™
    3 | void DoSomething(double a, bool useNewMode)
      |      ^~~~~~~~~~~
ambiguous.cxx:2:6: note: candidate: ā€˜void DoSomething(double, double)ā€™
    8 | void DoSomething(double a, double multiplier)
      |      ^~~~~~~~~~~
2 Likes
5

Another minor aspect: In Qt/VTK apps, now that you can connect Qt signals to pointer-to-member-functions, Iā€™ve sometimes found it convenient to connect Qt signals directly to VTK functions.

Overloaded functions can be somewhat awkward in that context though, even if Qt provides a helper function template to disambiguate (qOverload).

But yea, not a big issue. Just thought I should mention it since Qt itself recommends against adding overloads to slots in the library for this reason.

1 Like
6

Thanks for all the responses! I think we are getting closer to improving recommendations. Based on these, I would suggest this change:

Current:

  1. Prefer overloading functions to default arguments.
    Rationale: Default function arguments in C++ are a tempting way to add an argument to a function while maintaining easy backwards compatibility. However, if you later want to add another argument to the list in a way that preserves backwards compatibility, it, too, must be a default argument. To supply the second of these arguments in a call forces you to also supply the first argument, even if it is the default value. As a result, this is not a clean way to add a argument to a function. Instead, function overloading should be preferred.

Proposed:

  1. Avoid default arguments and function overloading
    Rationale: Default function arguments and function overloading in C++ are tempting ways to add an argument to a function while maintaining backwards compatibility, but both options have several disadvantages (see for example Google C++ style guide). Instead of adding more function arguments, it is preferable to rely on class member variables, which are initialized to sensible defaults in the constructor and can be get/set individually.

Rationale:

  • The choice between default values/overloads is controversial. Some very smart people completely disagree (e.g., smart cpp blogger vs. cpp core guidelines author), and Google style guide and people in this topic bring up several pros and cons for both options. The current recommendation of ā€œfunction overloading should be preferredā€ is oversimplified, confusing, biased advice.
  • Growing number of function parameters is a valid problem, but both overloads and default arguments are about equally bad solutions. VTK convention of using methods with minimal number of input arguments (e.g., Update()) and relying on member variables (that are initialized to sensible defaults and can be get/set individually) to specify inputs is a much better solution to this problem.