1

Im not sure if this is a stupid question, so shoot me if it is!

I am having this "dilemna" which I encounter very often. I have say two overloaded functions in C++

say we have this two overloads of F (just a pseudocode below)

void F(A a, .../*some other parameters*/)
{ 
  //some code part
  //loop Starts here
  G1(a,.../* some other parameter*/)
  //loop ends here
  //some code part
}


void F(B b, .../*some other parameters*/)
{
  //some code part
  //loop Starts here
  G2(b,.../* some other parameter*/)
  //loop ends here
  //some code part
}

where A and B are different types and G1 and G2 are different functions doing different things. The code part of the overloads except for G1 and G2 lines are the same and they are sometimes very long and extensive. Now the question is.. how can I write my code more efficiently. Naturally I want NOT to repeat the code (even if it's easy to do that, because its just a copy paste routine). A friend suggested macro... but that would look dirty. Is this simple, because if it is Im quite stupid to know right now. Would appreciate any suggestions/help.

Edit: Im sorry for those wanting a code example. The question was really meant to be abstract as I encounter different "similar" situation in which I ask myself how I am able to make the code shorter/cleaner. In most cases codes are long otherwise I wouldn't bother asking this in the first place. As KilianDS pointed out, it's also good to make sure that the function itself isn't very long. But sometimes this is just unavoidable. Many cases where I encounter this, the loop is even nested (i.e. several loops within each other) and the beginning of F we have the start of a loop and the end of F we end that loop.

Jose

Jose
  • 611
  • 1
  • 9
  • 16
  • 2
    Is there a reason that your "`some code part`" can't be made into a separate method? Without knowing what the code is it's hard to comment. – Dan Puzey Oct 23 '12 at 09:40
  • Template? Inheritance + virtual member function? – BoBTFish Oct 23 '12 at 09:40
  • Same as @Dan Puzey: too little details to give an opinion – Vincent Mimoun-Prat Oct 23 '12 at 09:41
  • 1
    How long exactly is the "other code"? It could be your function simply shouldn't be that long at all, independent of this problem. – KillianDS Oct 23 '12 at 09:44
  • 1
    Just for fun, let's throw another method into the round: Function Pointers! – stefan Oct 23 '12 at 09:45
  • http://stackoverflow.com/questions/1174169/function-passed-as-template-argument – FrankH. Oct 23 '12 at 09:45
  • @FrankH. Good _related_ question. In the case here, we can easily use the difference in the type. – stefan Oct 23 '12 at 09:47
  • @stefan: yes, I'm not implying it's the only solution. But the problem is related, and templating is one possible solution. In a way, this is a _compose_ problem, http://www.josuttis.com/cppcode/compose.html - left out of the STL for reasons I'm not sure about. – FrankH. Oct 23 '12 at 10:05

2 Answers2

7

A simple way to prevent the code duplication in this case would be to use a template. E.g:

void G(A a,.../* some other parameter*/)
{
    G1(a,.../* some other parameter*/);
}
void G(B b,.../* some other parameter*/)
{
    G2(b,.../* some other parameter*/);
}

template <typename T>
void F(T x, .../*some other parameters*/)
{ 
  //some code part
  //loop Starts here
  G(x,.../* some other parameter*/)
  //loop ends here
  //some code part
}

Note how the overloaded G function is used to determine whether to call G1 or G2. However, also note that this only prevents the code duplication, not the duplication in the compiled executable (because each template instantiation creates its own code).

Depending on the surrounding architecture there might be a number of other viable options (e.g. virtual methods instead of G1/G2 calls, function pointers, lambda functions if you have C++11...)

codeling
  • 11,056
  • 4
  • 42
  • 71
  • Ok. so I guess if G takes n different types then Ill have to make a template with n types and then call G – Jose Oct 23 '12 at 10:02
  • @Jose If the types are the same for both functions, you don't need to make them template arguments. – James Kanze Oct 23 '12 at 10:13
3

The most obvious solution is to put the common code parts into separate functions, and call them:

void F( A a, ... )
{
    commonPrefix(...);
    G1( a, ... );
    commonPostfix(...);
}

void F( B b, ... )
{
    commonPrefix(...);
    G2( a, ... );
    commonPostfix(...);
}

if there is a lot of data shared between the prefix and the postfix, you could create a class to hold it, and make the functions members.

Alternatively, you could forward to a template, possibly using traits:

template <typename T>
class GTraits;

template<>
class GTraits<A>
{
    static void doG( A a, ... ) { G1( a, ... ); }
};

template <>
class GTraits<B>
{
    static void doG( B b, ... ) { G2( b, ... ): }
};

template <typename T>
void doF( T t, ... )
{
    //  ...
    GTraits<T>::doG( t, ... );
    //  ...
}

void F(A a, ...)
{
    doF( a, ... );
}

void F(B b, ...)
{
    doF( b, ... );
}

This could easily result in the common parts of the code being duplicated, however. (Whether this is a problem or not depends. In most cases, I suspect that the code bloat would be minimal.)

EDIT:

Since you say that the common code includes the loop logic: you can use the template method pattern, something like:

class CommonBase
{
    virtual void doG( other_params ) = 0;
public:
    void doF()
    {
        //  prefix
        //  loop start
        doG( other_params );
        //  loop end
        //  suffix
    }
};

You then define a separate derived class in each function:

void F( A a ,... )
{
    class DoG : public CommonBase
    {
        A myA;
        void doG( other_params ) { G1( myA, other_params ); }
    public:
        DoG( A a ) : myA( a ) {}
    } do( a );
    do.doF();
}

The fact that you need the forwarding constructor makes it a bit wordy, but it does keep all of the common code common.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
  • this is the initial thought. But what hinders it (or what I thought would give me the problem) is that I have the code prefix telling me to start a loop and the postfix tells me to end a loop (I could even be whitin a very complicated nested loop). Would this method still work for that? – Jose Oct 23 '12 at 10:21
  • If you mean the first method, not directly. It could be done by using the template method pattern, however. I'll edit my response to provide an example. – James Kanze Oct 23 '12 at 10:26
  • Well I guess I will choose the method with inheritance if I want to export the function in a library (and still want to keep the code to myself). If I want a faster runtime then I would choose the template solution. Nice ideas from everyone thanks a lot! – Jose Oct 23 '12 at 11:24