When should code that looks like:
DoThing(string foo, string bar);
DoThing(string foo, string bar, int baz, bool qux);
...
DoThing(string foo, string bar, int baz, bool qux, string more, string andMore);
Be refactored into something that can be called like so:
var doThing = new DoThing(foo, bar);
doThing.more = value;
doThing.andMore = otherValue;
doThing.Go();
Or should it be refactored into something else entirely?
In the particular case that inspired this question, it’s a public interface for an XSLT templating DLL where we’ve had to add various flags (of various types) that can’t be embedded into the string XML input.
1
I believe the common approach is to encapsulate some (or all) of the parameters into a class and have that as a parameter:
DoThing(string foo, string bar, DoThingParameters parameters = null);
If you use C#, you can then write those parameters directly inline when calling the method:
DoThing(foo, bar, new DoThingParameters { More = value, AndMore = otherValue });
Both work, though I personally dislike assigning public members or properties like that if these members or properties are used directly for one purpose.
In other words, if you have a logger class, you shouldn’t have to assign “logger.message” and then call “log()”, since message is used strictly for the log method, hence it should be passed.
Understandably, I can see the need for doing such things in order to avoid refactoring code. Here are a couple things you could do instead:
- Consider passing non-specific parameters in the constructor rather than the method itself. What I mean by non-specific parameters are parameters which aren’t specific to a particular method but control behavior that isn’t likely to change in the instance. Returning to the logger class example, such a parameter might be the directory where the log file is saved.
- Consider passing a key-value map containing all parameters. This is the preferred method for passing non-descript parameters to third party libraries anyway. If you find yourself dealing with large amounts of parameters to handle, then this might be preferable. A good example of this might be instantiating a database connection using many of the database-specific parameters rather than having to have a static method for each database. The method can then pull whatever parameters it finds that it can use. However, I recommend that if you do it this way that you consider eliminating the other method signatures and forcing all calls to use this key-value system for consistency.
I hope that helps!
1
What you are facing here is the telescoping constructor anti-pattern.
IMHO you should refactor to public members only if it’s a simple class used to store data. Otherwise, your class looks like a good candidate for the Builder pattern. You can use it to:
-
encapsulate default member-values logic – set those values from inside the builder instead of setting them for each object you construct using it’s members public access
-
gain the ability to respond to inappropriate initialization – when the object is constructed from the builder you determine inside the builder what to do when not all required values were set
1