Alternatives to passing a flag into a method?

Sometimes when fixing a defect in an existing code base I might (often out of laziness) decide开发者_运维百科 to change a method from:

void
MyClass::foo(uint32_t aBar)
{
    // Do something with aBar...
}

to:

void
MyClass::foo(uint32_t aBar, bool aSomeCondition)
{
    if (aSomeCondition)
    {
        // Do something with aBar...
    }
}

During a code review a colleague mentioned that a better approach would be to sub-class MyClass to provide this specialized functionality.

However, I would argue that as long as aSomeCondition doesn't violate the purpose or cohesion of MyClass it is an acceptable pattern to use. Only if the code became infiltrated with flags and if statements would inheritance be a better option, otherwise we would be potentially be entering architecture astronaut territory.

What's the tipping point here?

Note: I just saw this related answer which suggests that an enum may be a better choice than a bool, but I think my question still applies in this case.

There is not only one solution for this kind of problem.

Boolean has a very low semantic. If you want to add in the future a new condition you will have to add a new parameter...
After four years of maintenance your method may have half a dozen of parameters, if these parameters are all boolean it is very nice trap for maintainers.

Enum is a good choice if cases are exclusive. Enums can be easily migrated to a bit-mask or a context object.

Bit mask : C++ includes C language, you can use some plain old practices. Sometime a bit mask on an unsigned int is a good choice (but you loose type checking) and you can pass by mistake an incorrect mask. It is a convenient way to move smoothly from a boolean or an enum argument to this kind of pattern. Bit mask can be migrated with some effort to a context-object. You may have to implement some kind of bitwise arithmetics such as operator | and operator & if you have to keep a buildtime compatibility.

Inheritence is sometime a good choice if the split of behavior is big and this behavior IS RELATED to the lifecycle of the instance. Note that you also have to use polymorphism and this is may slow down the method if this method is heavily used.
And finally inheritence induce change in all your factory code... And what will you do if you have several methods to change in an exclusive fashion ? You will clutter your code of specific classes... In fact, I think that this generally not a very good idea.

Method split : Another solution is sometime to split the method in several private and provide two or more public methods.

Context object : C++ and C lack of named parameter can be bypassed by adding a context parameter. I use this pattern very often, especially when I have to pass many data across level of a complex framework.

class Context{
public:
  // usually not a good idea to add public data member but to my opinion this is an exception
  bool setup:1; 
  bool foo:1;
  bool bar:1;
  ...
  Context() : setup(0), foo(0), bar(0) ... {}
};
...    

Context ctx;
ctx.setup = true; ...
MyObj.foo(ctx);

Note: That this is also useful to minimize access (or use) of static data or query to singleton object, TLS ... Context object can contain a lot more of caching data related to an algorithm. ... I let your imagination run free...

Anti patterns

I add here several anti pattern (to prevent some change of signature): *NEVER DO THIS *

• *NEVER DO THIS * use a static int/bool for argument passing (some people that do that, and this is a nightmare to remove this kind of stuff). Break at least multithreading...
• *NEVER DO THIS * add a data member to pass parameter to method.

Unfortunately, I don't think there is a clear answer to the problem (and it's one I encounter quite frequently in my own code). With the boolean:

 foo( x, true );  

the call is hard to understand .

With an enum:

 foo( x, UseHigherAccuracy );

it is easy to understand but you tend to end up with code like this:

 foo( x, something == someval ? UseHigherAccuracy : UseLowerAccuracy );

which is hardly an improvement. And with multiple functions:

 if ( something == someval ) {
      AccurateFoo( x );
 }
 else {
      InaccurateFoo( x );
 }

you end up with a lot more code. But I guess this is the easiest to read, and what I'd tend to use, but I still don't completely like it :-(

One thing I definitely would NOT do however, is subclass. Inheritance should be the last tool you ever reach for.

The primary question is if the flag affects the behaviour of the class, or of that one function. Function-local changes should be parameters, not subclasses. Run-time inheritance should be one of the last tools reached for.

The general guideline I use is: if aSomeCondition changes the nature of the function in a major way, then I consider subclassing.

Subclassing is a relatively large effort compared to adding a flag that has only a minor effect.

Some examples:

• if it's a flag that changes the direction in which a sorted collection is returned to the caller, that's a minor change in nature (flag).
• if it's a one-shot flag (something that affects the current call rather than a persistent change to the object), it should probably not be a subclass (since going down that track is likely to lead to a massive number of classes).
• if it's a enumeration that changes the underlying data structure of your class from array to linked list or balanced tree, that's a complex change (subclass).

Of course, that last one may be better handled by totally hiding the underlying data structure but I'm assuming here that you want to be able to select one of many, for reasons such as performance.

IMHO, aSomeCondition flag changes or depends on the state of current instance, therefore, under certain conditions this class should change its state and handle mentioned operation differently. In this case, I can suggest the usage of State Pattern. Hope it helps.

I would just change code:

void MyClass::foo(uint32_t aBar, bool aSomeCondition)
{
    if (aSomeCondition)
    {
        // Do something with aBar...
    }
}

to:

void MyClass::foo(uint32_t aBar)
{
    if (this->aSomeCondition)
    {
        // Do something with aBar...
    }
}

I always omit bool as function parameter and prefer to put into struct, even if I would have to call

myClass->enableCondition();