33

I am implementing a DelegateCommand, and when I was about to implement the constructor(s), I came up with the following two design choices:

1: Having multiple overloaded constructors

public DelegateCommand(Action<T> execute) : this(execute, null) { } public DelegateCommand(Action<T> execute, Func<T, bool> canExecute) { this.execute = execute; this.canExecute = canExecute; }

2: Having only one constructor with an optional parameter

public DelegateCommand(Action<T> execute, Func<T, bool> canExecute = null) { this.execute = execute; this.canExecute = canExecute; }

I don't know which one to use because I don't know what possible advantages / disadvantages come with either of the two proposed ways. Both can be called like this:

var command = new DelegateCommand(this.myExecute); var command2 = new DelegateCommand(this.myExecute, this.myCanExecute);

Can someone please point me in the right direction and give feedback?

Improve this question
9
  • 4
    KISS and YAGNI. In case of doubts, implement both. After a while, do a search of references to both constructors. It would not surprise me if one of them was never consumed externally. Or marginally consumed. This is something easy to find performing static analysis of the code. Commented Mar 27, 2018 at 9:26
  • 3
    Keep code analysis rule CA1026: Default parameters should not be used in mind. Commented Mar 27, 2018 at 18:27
  • 2
    @Laiv am I missing something? They're used in the same way and thus have the same signature. You can't search for references and tell which the caller was thinking of. It sounds more like you're referring to whether or not OP should even have that second argument, but that's not what OP is asking about (and they may well know for sure that they need both sometimes, hence them asking). Commented Mar 27, 2018 at 19:57
  • 2
    @Voo The compiler for Openedge|Progress 10.2B ignores them. Pretty much killed our non-breaking method change strategy; instead, we needed to use overloading for the same effect. Commented Mar 27, 2018 at 20:00
  • 2
    @Kat, most of the constructors are declared during the early stages of the development. Many are declared with no certainity of usefulness. That's why I suggested to keep working and perform analysis later. Don't get stuck in these sort of issues because which ones is better for you is something you will see after a while. Code change and change often. At the early phases of the SDLC, any advantage/disavantage remains to be seen. Overall at such low level of the implementation. In case of doubts keep It simple. Commented Mar 27, 2018 at 20:53

3 Answers 3

Reset to default
25

I prefer multiple constructors over default values and personally I don't like your two constructor example, it should be implemented differently.

The reason for using multiple constructors is that the main one can just check if all parameters are not null and whether they are valid whereas other constructors can provide default values for the main one.

In your examples however, there is no difference between them because even the secondary constructor passes a null as a default value and the primary constructor must know the default value too. I think it shouldn't.

This means that it would be cleaner and better separated if implemented this way:

public DelegateCommand(Action<T> execute) : this(execute, _ => true) { } public DelegateCommand(Action<T> execute, Func<T, bool> canExecute) { this.execute = execute ?? throw new ArgumentNullException(..); this.canExecute = canExecute ?? throw new ArgumentNullException(..); }

notice the _ => true passed to the primary constructor that is now also checking all parameters for null and doesn't care about any defaults.

The most important point however is extensibility. Multiple constructors are safer when there is a possibility that you will extend your code in the future. If you add more required parameters, and the optional ones must come at the end, you'll break all your current implementations. You can make the old constructor [Obsolete] and inform the users that it's going to be removed giving them time to migrate to the new implementation without instantly breaking their code.

On the other hand making too many parameters optional would be confusing too because if some of them are required in one scenario and optional in another one you would need to study the documentation instead of just picking the right constructor by simply looking at its parameters.

Improve this answer
8
  • 16
    On the other hand making too many parameters optional would be confusing... To be honest, having too many parameters (regardless if they're optional or not) is confusing. Commented Mar 27, 2018 at 7:39
  • 1
    @DavidPacker I agree with you, but how many parameters are too many is another story, I think ;-) Commented Mar 27, 2018 at 7:43
  • 2
    There is another difference between having a constructor that omits a parameter versus a constructor that has a default for the parameter. In the first case, the caller can explicitly avoid passing the parameter, whereas in the latter case the caller cannot - because passing no parameter is the same as passing the default value. If the constructor needs to make this distinction, then two constructors is the only way to go. Commented Mar 27, 2018 at 15:32
  • 1
    For example, suppose I have a class that formats strings, which I can optionally construct with a CultureInfo object. I would prefer an API that allowed the CultureInfo parameter to be supplied via an additional parameter, but insisted that if it was supplied then it should not be null. That way accidental null values don't get mis-interpreted as meaning "none". Commented Mar 27, 2018 at 15:36
  • I'm not sure extensibility is really a reason against optional parameters; if you have required parameters at all and ever change the number of them you will have to create a new constructor and Obsolete the old one if you want to avoid breaking things, whether you also have optional parameters or no. With optional parameters you only have to Obsolete if you remove one. Commented Mar 27, 2018 at 18:11
17

Considering the only thing you're doing in the constructor is simple assignment, the single constructor solution may be a better choice in your case. The other constructor provides no extra functionality and it's clear from the design of the constructor with two parameters that the second argument does not need to be supplied.

Multiple constructors do make sense when you're constructing an object from different types. In that case, the constructors are a substitution of an external factory because they process the input parameters and format them to a correct internal property representation of the class which is being constructed. That does not however happen in your case hence why a single constructor should be more than enough.

Improve this answer
2

I think there's two different cases for which each approach can shine.

Firstly, when we have simple constructors (which is usually the case, in my experience), I would consider optional arguments to shine.

  1. They minimize code that has to be written (and thus also that has to be read).
  2. You can ensure that documentation is in one place and isn't repeated (which is bad because it opens up an extra area that could get outdated).
  3. If you have many optional arguments, you can avoid having a very confusing set of combinations of constructors. Heck, even with just 2 optional arguments (unrelated to each other), if you wanted separate, overloaded constructors, you'd have to have 4 constructors (version without any, version with each, and version with both). This obviously doesn't scale well.

Buuuut, there are cases where optional arguments in constructors just make things clearly more confusing. The obvious example is when these optional arguments are exclusive (ie, can't be used together). Having overloaded constructors that only permit the actually valid combinations of arguments would ensure compile time enforcement of this constraint. That said, you should also avoid even encountering this case (eg, with multiple classes inheriting from a base, where each class does the exclusive behavior).

Improve this answer
1
  • +1 for mentioning the permutation explosion with multiple optional parameters. Commented Sep 9, 2018 at 13:13