Interpretation of DRY principle

Question

Right now I'm struggling with this concept of DRY (Don't Repeat Yourself) in my coding. I'm creating this function in which I'm fearing it's becoming too complex but I'm trying to follow the DRY principle.

createTrajectoryFromPoint(A a,B b,C c,boolean doesSomething,boolean doesSomething2)

This function I have say takes 3 input parameters, and then the function will do something slightly different given the boolean combinations doesSomething and doesSomething2. However the problem I'm having is that this function is growing in complexity greatly with every new boolean parameter that's added.

So my question is, is it better to have a bunch of different functions that share a lot of the same logic (therefore violating the DRY principle) or one function that behaves slightly differently given a number of parameters but making it much more complex (but preserving DRY)?

Answer 1

boolean arguments to trigger different code paths in a single function/method is a terrible code smell.

What you are doing violates Loose Coupling and High Cohesion and Single Responsibility principles, which are much more important than DRY in precedence.

That means that things should depend on other things only when they have to ( Coupling ) and that they should do one thing and only one thing ( very well ) ( Cohesion ).

By your own omission, this is too tightly coupled ( all the boolean flags are a type of state dependency, which is one of the worst! ) and has too many individual responsibilities intermingled ( overly complex ).

What you are doing is not in the spirit of DRY anyway. DRY is more about repetition ( the R stands for REPEAT ). Avoiding copy and pasting is its most basic form. What you are doing isn't related to repetition.

Your problem is your decomposition of your code isn't at the correct level. If you think you will have duplicate code, then that should be its own function/method that is parameterized appropriate, not copy and pasted, and the others should be named descriptively and delegate to the core function/method.

Answer 2

The fact that you are passing in booleans to make the function do different things is a violation of the Single Responsibility Principle. A function should do one thing. It should do one thing only, and it should do it well.

It smells like you need to split it into several smaller functions with descriptive names, separating the code paths corresponding to the values of those booleans.

Once you do that you should look for common code in the resulting functions, and factor it out into its own function(s). Depending on how complex this thing is, you may even be able to factor out a class or two.

Of course, this assumes that you are using a version control system, and that you have a good test suite, so that you can refactor without fear of breaking something.

Answer 3

Why won't you create another function containing all the logic in your function before you decide to do something or something2 and then have three functions like :

createTrajectoryFromPoint(A a,B b,C c){...}

dosomething(A a, B b, C c){...}

dosomething2(A a, B b, C c){...}

And now by passing three same parameter types to three different functions, you will be again repeating yourself, so you should define a struct or class containing A,B,C.

Alternatively you can create a class containing parameters A, B, C and a list of operations to be done. Add which operations(something, something2) you want to happen with these parameters (A,B,C) by registering operations with the object. Then have a method to call all registered operations on your object.

public class MyComplexType
{
    public A a{get;set;}
    public B b{get;set;}
    public C c{get;set;}

    public delegate void Operation(A a, B b, C c);
    public List<Operation> Operations{get;set;}

    public MyComplexType(A a, B b, C c)
    {
        this.a = a;
        this.b = b;
        this.c = c   
        Operations = new List<Operation>();
    }

    public CallMyOperations()
    {
        foreach(var operation in Operations)
        {
            operation(a,b,c);
        }
    }
}

Answer 4

DRY can be taken too far, its best to use the single responsibility principle (SRP) in conjunction with DRY. Adding bool flags to a function to make it do slightly different versions of the same code can be a sign you are doing too much with one function. In this case I would suggest creating a separate function for each case that your flags represent, then when you have each function written out it should be fairly apparent if there is a common section that can be moved to a private function without passing all the flags, if there isn't an apparent section of code, then you really aren't repeating yourself, you have several different but similar cases.

Answer 5

I usually go through several steps with this problem, stopping when can't figure out how to go further.

First, do what you have done. Go hard with DRY. If you do not end up with a big hairy mess, you're done. If, as in your case, you have no duplicate code but each boolean has its value checked in 20 different places, go to the next step.

Second, split the code into blocks. The booleans are each referenced only once (well, maybe twice sometimes) to direct execution to the right block. With two booleans, you end up with four blocks. Each block is almost identical. DRY is gone. Do not make each block a separate method. That would be more elegant, but putting all the code in one method makes it easier, or even possible, for anyone doing maintenance to see that they have to make each change in four places. With well organized code and a tall monitor, the differences and mistakes will be almost obvious. You now have maintainable code and it will run faster than the original tangled mess.

Third, try to grab duplicate lines of code from each of your blocks and make them into nice, simple methods. Sometimes you can't do anything. Sometimes you can't do much. But every little bit you do moves you back towards DRY and makes the code just a bit easier to follow and safer to maintain. Ideally, your original method might end up with no duplicate code. At that point, you may want to split it into several methods without the boolean parameters or you may not. The convenience of the calling code is now the main concern.

I added my answer to the large number already here because of the second step. I hate duplicate code, but if it's the only intelligable way to solve a problem, do it in such a way that anyone will know at a glance what you are doing. Use multiple blocks and only one method. Make the blocks as identical as possible in names, spacing, alignments, ...everything. The differences should then jump out at the reader. It might make it obvious how to rewrite it in a DRY manner, and if not, maintaining it will be reasonably straightforward.

Answer 6

An alternative approach is to replace the boolean parameters with interface parameters, with code to handle the different boolean values refactored into the interface implementations. So you would have

createTrajectoryFromPoint(A a,B b,C c,IX x,IY y)

where you have implementations of IX and IY that represent the different values for the booleans. In the body of the function, wherever you have

if (doesSomething)
{
     ...
}
else
{
     ...
}

you replace it with a call to a method defined on IX, with the implementations containing the omitted code blocks.