code style: generalize at bottom for different clauses?

Question

I have a for loop with a long if ... else if chain inside, and inside each of the cases is a clause. Some of these clauses can be really long, but have some code that is present in other clauses. Should I take the code that is in the code in the other clauses that do the same thing as the other clauses and put them at the end of the if ... else if chain and put CONTINUEs in clauses that don't use this code, or should I just leave it?

For example,

for {

    if (...) {
       // clause1: has some code that matches clause2, and others but not all clauses
    } else if (...) {
        // clause2: has some code that matches clause1
    } else if (...) {
        // clause3: no code matches clause1
    }

    // maybe generalize clause1 and clause2 code and put here? and put a CONTINUE in the third clause/clauses that don't have this generalizable code?

}

Answer 1

I suggest simplifying the code as much as possible, so that it is easier to understand. Exactly what that means will depend on the content of your clauses.

For example, it might be clearest to pull out code as you suggest, although having a combination of continue vs. not-continue clauses has the potential to confuse readers of the code. Another alternative might be to extract the repeated code into a separate method and call it where necessary, which has the advantage that you can give it a meaningful name.

However, you've described this code as a long if ... else if chain with long clauses. This is a code smell and suggests that deeper refactoring might be necessary for it to be maintainable. For example, consider using the strategy pattern.