What should I consider when adding a condition to an existing function?

Question

I often find that I need to add new conditional feature to code that already exists. This requires me to keep the existing functionality while tacking on something new. I've tried each of the following methods but neither of them seems as elegant as I would like.

Option 1:

bar_1():
    baz()
    frobnicate()

bar_2():
    baz()
    defrobnicate()

//Call functions.
if(foo == True):
    bar_1()
else:
    bar_2()

Option 2:

bar(foo):
    baz()
    if(foo):
        frobnicate()
    else:
        defrobnicate()

//Call the one function.
bar(foo)

In option 1, the baz() call is repeated. Not a big deal in this example but in actual code that can be 30 lines of repeating yourself.

In option 2, the code is more compact, but now we have complicated all calls to bar with this extra parameter and added conditionals to our function. And in real code these conditionals can appear in several places making the function more difficult to read.

Are there other considerations I'm missing in deciding which to use?

Possible duplicate of [If Else - Repeated Code Logic](https://softwareengineering.stackexchange.com/questions/275445/if-else-repeated-code-logic) — gnat, Oct 20 '17 at 21:10
Since you've used foo bar examples that don't characterize the functions in any way, the only thing we can tell you is which arrangement is most likely to produce a favorable result, which would be option 1. — Robert Harvey, Oct 20 '17 at 22:04
Actually, I'm going to go out on a limb and say which approach you select is going to depend *almost entirely* on the nature of the functions, in which case we can't advise you regarding your generic example in any meaningful way. There are too many "variables" that are not considered. — Robert Harvey, Oct 20 '17 at 22:06
Robert Harvey is right, the elephant in the room is you asked this in terms of "foo" and "bar". But the topmost consideration you should do here is *"which abstractions do I want to build"* - which can only be answered by knowing the real function names and the real semantics of your functions. — Doc Brown, Oct 21 '17 at 05:42

score 1 · Answer 1 · answered Oct 20 '17 at 21:12

1

Is the conditional you are adding ACTUALLY part of the function?

If you need to put the conditional around every call to the function, you write a wrapper for it that applies the conditional.

answered Oct 20 '17 at 21:12

John R. Strohm

18,043
5
46
56

Yes the conditional would be part of the function. Maybe I oversimplified in my example. Let's assume the conditional affects some other aspect of the function, such as filters applied to a database query. – Bindelstif Oct 20 '17 at 21:21
This is the third option and is the best. Perhaps it should be more clear that the "wrapper" is a function. – Frank Hileman Oct 20 '17 at 22:47
By ACTUALLY part of the function, I meant "Does it inherently relate to the purpose of the function?". If the function, say, controls how the system forms a complex signal sequence, and the condition is a business rule about WHEN to emit that pulse sequence, then it is NOT actually part of the function. – John R. Strohm Oct 21 '17 at 04:04

score 1 · Answer 2 · answered Oct 20 '17 at 23:11

I guess it depends on what language constructs are available to you. If this were, say, c#, I might do it like this:

class FooBase
{
    virtual void Baz()
    {
        //Do Baz stuff
    }

    abstract DoWork();

    public void Bar()
    {
        Baz();
        DoWork();
    }
}

Then implement a subclass for each option

class Foo1 : FooBase
{
    override void DoWork()
    {
        //Do frobnicate stuff
    }
}

class Foo2 : FooBase
{
    override void DoWork()
    {
        //Do defrobnicate stuff
    }
}

Then in your main program:

FooBase e = foo ? new Foo1() : new Foo2();
e.Bar();

This guarantees that Baz is always called before doing the work (frobnicate or defrobnicate) which addresses the problem of sequential coupling.

Meanwhile you can provide as many subclasses of FooBase as there are conditions, so it is extensible.

And finally, if you ever need to, you can override Baz for any of the specific conditions, in case you find they need slightly different initialization logic.

If your functions share state variables, they can be implemented as private class member variables, which helps control your scope (which is a bit messy from the sounds of it) and clarifies where the dependencies are.

But again, depends on your language. If you're writing this in assembly you might want to avoid using a class-based approach.

It would be good to point out to the OP that this is called "polymorphism" in case they wish to study it in more depth. — user1118321, Oct 21 '17 at 02:09
Technically, this is an option. But to tell this someone who believes such a question can seriously be answered in terms of foo/bar examples is like telling one to use a chainsaw when he only wants to know which knife he should use (with the additional recommandation, only use a chainsaw if your environment provides one). — Doc Brown, Oct 21 '17 at 05:38

score 1 · Answer 3 · answered Oct 21 '17 at 07:04

This is a classic example of single responsibility principle on the method level. Often you start with a simple method having none or a few arguments and as your application ages, you end up with a monster method having 10+ arguments and a ton of if-then constructs. If you don't know what you are doing that is. Apparently you sense there is something wrong here at an early stage. Good!

What is going on here is you have some additional complexity creeping in. That means it is time to reconcider your design (on that method level). Consider what you can separate, either with polymorphism of an additional Foo-variety method that you may want to call instead of the one you have. The point is to get to the point again where you have less cyclomatic complexity in your individual methods. You do this by singeling out responsibilities in separate methods or classes.

score 0 · Answer 4 · answered Oct 20 '17 at 21:34

In most cases, I would recommend the first option. If baz() is actually 30 lines of code you don't want to write and maintain twice, then separating that into its own function is reasonable and you barely have any extra repeated code. Then, you get the advantage that you didn't make a breaking change and all existing code will continue to work; you just have a second option if you need it.

If the difference that foo makes is more convoluted than in the example, affecting things interspersed throughout baz(), then it would certainly be more difficult and/or less effective to pull out into a shared function. On the other hand, if the differences are that ingrained and vital to the entire operation of the function then that's actually an even BIGGER reason to have them be separate functions. Functions which do different things should not be the same function.

score 0 · Answer 5 · answered Oct 21 '17 at 14:16

If this kind of a problem happens often in the code ("I often find that I need to add new conditional feature to code that already exists."), then the code is not well-designed or it's design is not understood enough.

In general, it is very hard to give an advice.

First, you should start maintain code less surprising: If you do not have time/money/inclination to refactor, then continue using the same ways as were used before, do not introduce obscure ways. (it's easy, for example do something like:

{True: bar_1, False: bar_2}[foo]()

or do the same with a cool hierarchy of Frobnicator classes, which will solve dispatch problems for you, but this coolness does not mean code will become more maintainable.

Second, consider the business domain. Does foo really belongs to concerns of the bar? Does it belong to the same level of abstraction? For the code to be cleaner, function should make only one thing, so each provided parameter makes less cleaner code. Good indicator is, whether the bar_1 and bar_2 have their own distinguishable interpretation in the domain model. If, they do, do not hide if in the function. If the foo is minor parameter, this if be better inside. (The only exception to this I see in scientific modeling software, where functions can have a lot of parameters and a different programming culture). Also, consider how many times the function is called, because each call will need to be touched everywhere to change the function signature.

If you have doubts after the above, one more consideration can be if you have more than one foo-like parameters. You can ask yourself a question: are those parameters truly independent or possibly belong as attributes to some entity? For example, if you have x, y parameters and add z, you can consider to use an object point instead. It may then well be, that the choice will become a concern of that new entity.

As the last defense, you may be better off using rules engine or some other declarative approach. It may keep complexity low and easily maintainable.

Even though you have not asked, making logic more complex with ifs may backfire. Tracing the logic will become harder and harder, and the moment come where noone will understand what is going on, regardless on whether you have if inside or outside your function.

What should I consider when adding a condition to an existing function?

5 Answers5