Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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.

share|improve this question
2  
repetitive string foo, string bar are a violation of DRY principle. Extract class refactoring typically makes sense in cases like you describe –  gnat Oct 29 '13 at 10:18

3 Answers 3

up vote 2 down vote accepted

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 });
share|improve this answer

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:

  1. 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.
  2. 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!

share|improve this answer
    
Thanks. I was tempted to say that in a language with reasonable literal maps I'd just use Dothing({'foo': 'bar' ...), or of course named parameters. –  Ben Heley Oct 29 '13 at 11:26

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

share|improve this answer
    
That makes a lot of sense, though it's not actually a constructor (perhaps it should be, but that's another question). The telescoping constructor anti-pattern is definitely something I'll look into. –  Ben Heley Oct 31 '13 at 14:06

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.