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.

I have a style question about overloading methods/constructors. I have a constructor which does something very simple, and then calls a method with some side effects. Sometimes however I don't want the side effects so I made a separate overloaded constructor. I show two versions here. Version 1 is simple: if I pass any bool then TrickyFunction won't be called. But maybe that's confusing, because the value of no_tricks does not affect the behavior (only that no_tricks exists). Version 2 actually does process no_tricks and makes a decision to run TrickyFunction based on that.

Which version, or another version, is more stylistically "correct?"

public foo MyFunction(string a)
{
    variable = a
    TrickyFunction(a)
}

#Version 1
public foo MyFunction(string a, bool no_tricks)
{
    variable = a
}

#Version 2
public foo MyFunction(string a, bool no_tricks)
{
    variable = a
    if (notricks == true)
    {
        TrickyFunction(a)
    }
}
share|improve this question
    
BTW, there is no reason for == true, just write if (no_tricks). –  svick Jan 24 at 23:38
1  
huge code smell that I cannot comment on since none of the actual code is here. And by passing a bool to perform the same action a different way is revealing something about the internal workings of the function which is the obvious smell. If the method can produce different results (e.g. the side effects you mentioned) then it isn't the same method and shouldn't be an overload. Create a similarly named method where the name indicates that the results are a bit different. –  Sinaesthetic Jan 27 at 6:13

3 Answers 3

Version #2 is FAR preferable, although the overall design is often considered a code smell.

Note that more recent C# dialects allow for optional parameters, so you could combine your base and overload.

class foo {
  public foo(string a, bool no_tricks = false) {
    variable = a;
    if(no_tricks == false) {
      TrickyFunction(a);
    }
  }
}

(And, of course, note that passing a false value to override default is kinda counter-intuitive by itself, especially when your logical value doens't match the name of the variable, as in your text.)

share|improve this answer

Version 2, kinda.

I would only have one constructor, but the second parameter doExtraWork is optional, and defaulted to true. This follows the general guideline that boolean true should represent doing things rather than not doing things. And this approach would simplify your code. If optional params are forbidden in your codebase, then just have the "little" constructor call the other.

Though I'd look to remove tricky stuff from any constructor - do just enough to initialize your invariants.

share|improve this answer

While C# does not allow a type to have multiple constructors except by giving them all different signatures, types can have any number of factory methods. My inclination would be to have a protected constructor with a "noTricks" parameter, but have consumers of the class use factory methods to construct instances. Tricky and non-tricky methods can then be given different names to clarify their behavior.

share|improve this answer

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.