Is there an elegant way to write code with differing paths having a common middle segment?

Question

I started out with the following, simple routine:

void originalCode(){
    boolean status = getStatus();

    function2(status);

    if(status){
         return;
    }

    function3();    
}

But then, additional functionality required calling function1 on the status == true path. It was also important to call function1 before function2.

The results were one of the two following monstrosities:

void alternative1(){
    boolean status = getStatus();

    if(status){
        function1();
    }

    function2(status);

    if(status){
         return;
    }

    function3();    
}

or,

void alternative2(){
    boolean status = getStatus();

    if(status){
        function1();
        function2(status);
        return;
    }

    function2(status);
    function3();    
}

Both have code duplication, and in general, don't look right.

Now, in my particular case, function3 isn't required to be called after function2, so I settled on the following:

void niceAlternative(){
    boolean status = getStatus();

    if(status){
        function1();
    }else{
        function3();
    }

    function2(status);
}

But what if that was not an option--i.e. if function2 had to be called before function3? And in general, how do you structure code with multiple execution paths that have commonality in the middle?

Answer 1

Not every repeated identifier is a "code duplication". The indicator for code duplication is that when you want to make a change (renames don't count) to one occurrence of it, you usually have to make the exact same change to all occasions.

Calling function2(status); multiple times is not code duplication(Assuming, of course, it's really a function and not a placeholder for several lines of code. If that's the case you'll want to extract it to a function). It could have been code duplication if you had a long list of arguments, or if some of these arguments were compound. If that was the case, and you wanted to add/remove/change arguments, you would have to do it in all places the function is called. But with one argument, and a simple one too? Chances you'll want to change it are low.

Checking if(status) multiple times is not code duplication either. If instead of status you had some complex expression it would have been code duplication - but then you could have simply stored it in a variable.

So, you can pick both styles - but I suggest picking the one that checks status twice, because:

1. A function call - even a simple one - can still change if you need to add arguments. if(status) has much lower chance to change, because any possible change you could probably fit into its assignment statement.
2. The flow is clearer - function1, function2 and function3 are called in order, with function1 and function3 only called if status is true.

Answer 2

My first thought (I asked about it in a comment to the question) is, "what does function2 do differently based on the value of the input parameter?" I suspect that function2 has code that is inappropriately combined and maybe should be broken up. Even if this is not the case, I still would have more likely written the originalCode like this:

void originalCode() {
    if (getStatus()) {
        function2(true)
    }
    else {
        function2(false)
        function3()
    }
}

The motivation behind the above is that there are a set of side-effects for when the status is true and a different set of side-effects for when status is false. The structure above clearly tells the reader this at a glance. In other words, when the reader asks him(her)self, "What happens if status is true/false?" The answer is in a single obvious place and the reader doesn't need to scan the entire function to try to figure that out.

Also when the new requirement--that function1 be called before function2 and only if getStatus is true--comes in, the change is a simple addition:

void code() {
    if (getStatus()) {
        function1()
        function2(true)
    }
    else {
        function2(false)
        function3()
    }
}

The code still gives me a concise list of exactly what should happen if getStatus returns true and what should happen if it returns false.

Answer 3

Your description is rather abstract so will be my answer but hopefully shades some light on your question.

The example you provided is written in procedural style. In procedural programming, without using any remedy pattern, it can be unavoidable to have absolutely no code duplication. For e.g. C that's completely fine (although function tables exist) and could be even fine for object-oriented languages under the assumption that you don't expect that this piece of code going to change in the future. We had such a scenario for an industrial robot where the behavior was tied to the mechanical capabilities of the robot itself. Unless we changed the hardware, which we didn't, no need to change the code.

Generally if you have complex conditional logic you can remedy it with following approaches:

1. When you need to switch functionality based on some state variable, in object-oriented programming the State pattern could come in handy. Depending on the language you use it creates more or less boilerplate but is sometimes worth it. The pattern revolves around the notion of states in contrast to the next approach.
2. Build a Behavior tree that can be viewed as an execution plan. It's popular in game development where a state machine can cause a maintenance nightmare especially when additional functionality or intelligence is added over the time. The concept of the Behavior tree is based on actions instead of a state where each node incorporates an action to execute.
3. There's another method like list of function pointers (objects in OOP) for replacing long if-else chains or function tables.

Each method has its pros and cons and depends on your use case. Could be that none of them is practical in yours.

Regarding the necessity to add functionality as shown in alternative2 generally the Decorator or Composite pattern are nice solutions. In combination with the State pattern they could untangle the if-else construct.

But again, that's more of a list you can come back and evaluate whether it makes sense to employ one or a combination of these patterns. Some patterns carry significant weight like the State pattern or Behavior trees. As a rule of thumb I would stay pragmatic and go for niceAlternative and use a pattern if necessary or you already know that it's going to be enhanced over the time.