2

I have two for loops in a function which look like these:

for (int i=0; i < MAX; ++i) {
    identical_lines
    identical_lines
    identical_lines
    first_for_specific_line
    identical_lines
    identical_lines
    identical_lines
    first_for_specific_line
    identical_lines
    identical_lines
}

for (int i=MAX - 4; i > 0; --i) {
    identical_lines
    identical_lines
    identical_lines
    second_for_specific_line
    identical_lines
    identical_lines
    identical_lines
    second_for_specific_line
    identical_lines
    identical_lines
}

i.e. these two for loops have different conditions and indices, but their code is pretty much the same (all the 'identical_lines' are the same in the two for loops).

However there are a few spots (the one marked with 'specific_line') which are different for each one of these for loops.

I would like to avoid code duplication and merge these for loops but I can't think of something to unify them and still have the different lines.

The language I'm using is C++

  • 1
    Which C++ version? I think with a bit of functional mindset you can compose your algorithms using the specific lines as parametters. – Klaim May 8 '15 at 20:37
  • C++11 is fine to use. C++14 is a bit premature I suppose. I'm interested in that method, could you make it an answer or elaborate on that please? – Albert May 8 '15 at 20:46
  • 6
    Why not bring your real code to Code Review? – RubberDuck May 9 '15 at 0:09
10

Before grabbing into the "modern C++" bag, let us keep things simple and start with some classic, language agnostic techniques. One solution is to make a function of the form

   void myfunc(int i,bool upward)
   {
        identical_lines
        identical_lines
        identical_lines
        if(upward)
           first_for_specific_line
        else
           second_for_specific_line
        //...
    }

and call that like

 for (int i=0; i < MAX; ++i)
    myfunc(i,true);

  for (int i=MAX - 4; i > 0; --i) 
    myfunc(i,false);

Note that this is not always the best solution since it has a tendency to make myfunc doing "too much", maybe violating the single responsitibility principle. Often it is better to refactor each block you have marked as identical_lines into a small function on its own (lets call it block1(int i), block2(int i) , and create 2 functions

   myfunc1(int i)
   {
        block1(i);
        first_for_specific_line;
        block2(i);
        // ...
   }

and

   myfunc2(int i)
   {
        block1(i);
        second_for_specific_line;
        block2(i);
        // ...
   }

and use it like

  for (int i=0; i < MAX; ++i)
    myfunc1(i);

  for (int i=MAX - 4; i > 0; --i) 
    myfunc2(i);

However, to decide what is the "best" way to cut your code blocks down to small functions depends on what the blocks are really doing. It is better to build abstractions not primarily on formal criteria like "the code blocks look similar", but on "what is the task or purpose of this block. Ask yourself which of the lines belong together to fulfill that purpose and if you can characterize that task by a single, accurate name.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.