Methods should include the state needed to execute them as parameters

I’ve been seeing the following pattern in code that I’ve been working on lately.

var muhClass = new MuhClass();
muhClass.MuhProp = "foo";
muhClass.MuhOtherProp = "bar";

muhClass.MuhMethod();


public class MuhClass {

public string MuhProp {get; set; }
public string MyOtherProp {get; set; }

public void MuhMethod() {
  Console.WriteLine(MuhProp + MuhOtherProp);
}

This annoys me, I don’t like having to set properties before I call a method I would prefer the method signature to contain all of the data needed to execute the method.


public void MuhMethod(string muhProp, string MyOtherProp)

In fact, if you can use defaults use them, or use overloads.

public void MuhMethod(string muhProp, string MyOtherProp = "BallZacks")

public void MuMethod(string muhProp) 
{
    MuMethod(muhProp, "NutButter");
}
  • darrencauthon

    I agree. It causes confusion as to what is required for the method to run, as the properties set sort of pop out of thin air versus have them required in the function call. This pattern might make refactoring easier, as adding a property won’t stop a compiler, but that’s the sort of information you want to fail.

    It’s just awkward, anyway…. the user intends to do something, to call a function. Wrapping it in a class construct isn’t natural for that sort of action.

  • Pingback: Dew Drop – July 23, 2013 (#1,590) | Alvin Ashcraft's Morning Dew

  • Steve

    If those properties need setting for the object methods to work, then what you should really be doing is ensuring that the _constructor_ includes the state needed. The object should not be in an inconsistent state after construction (or when it’s been returned from the factory method that knows it has to invoke an initialization that can’t be called at construction time e.g. involving virtual method calls, or has the potential to fail messily leaving a broken construct).

    Alternatively, if the state is all pushed in from the outside for every method, then make those methods static — there’s no meaningful private state left on the object for them to be looking at or writing to at that point, after all.