How can I avoid this code duplication?

Question

I have two methods which have almost the same code except for two methods they call (and some other details I can easily parameterize). However, those method calls have the same signature, so I think I can generalize them into a single method.

class A{
    IApi* m_pApi;
    void M1();
    void M2();
public:
    void DoThings();
}

void A::M1(){
    int i;
    bool b;    
    m_pApi->method1( &i, &b );    
    //Other stuff...
}

void A::M2(){
    int i;
    bool b;    
    m_pApi->method2( &i, &b );
    //Other stuff...
}

void A::DoThings(){
    M1();
    M2();
}

I can figure out how to "parameterize" the "Other stuff" code, but the problem are the calls to method1 and method2. I think I have to use std::bind somehow, but I can't do something like this...

void A::M( std::function<void(int*,bool*)> f ){
    int i;
    bool b;
    f( &i, &b );
}

void A::DoThings(){
    M( std::bind( ???, m_pApi ) ); //M1
    M( std::bind( ???, m_pApi ) ); //M2
}

The problem here is that m_pApi is not a concrete class (it's an interface implemented by a bunch of concrete classes), so I'm not sure if I can do the usual &Class::Method thing. Suggestions?

Could you not just use an if statement and a boolean parameter flag? — qzcx, May 30 '14 at 19:58
Actually it's four methods (M1 ... M4). I could use an enum but I'm sure there's a more elegant solution. — dario_ramos, May 30 '14 at 19:59
Are you trying to just try a facade over the m_pApi class? This might help http://stackoverflow.com/questions/12662891/c-passing-member-function-as-argument — dre, May 30 '14 at 20:01
I also thought of using pointers to member functions, but I never used them when dealing with an inheritance hierarchy. If I use a `void (IApi::*)(int*, bool*)`, would it bind correctly when I combine it with `m_pApi` to perform the call? I'll go make a quick test at ideone. — dario_ramos, May 30 '14 at 20:05
@dario Yes, they work as they should (pointer to member of base can be applied to a derived instance). — Alan Stokes, May 30 '14 at 20:11
are you sure bind won't work? even if interface is not a concrete class, you are not really instantiating the object by binding stuff ... just a guess — Zeks, May 30 '14 at 20:12
@Zeks: I think pointers to members fit this scenario better, since they are less generic and thus give more information to those who read the code, and better type safety as well. In other words, laziness > curiosity :P — dario_ramos, May 30 '14 at 20:16
sorry, wrong test case http://codepad.org/Oj08FpUS this compiles in qt and outputs "1 called" and "2 called" — Zeks, May 30 '14 at 20:24

score 3 · Accepted Answer · answered May 30 '14 at 20:09

Use pointers to member function.

#include <iostream>
using namespace std;

struct IApi {
    void method1(int * i, bool * b) {
        *i = 1; *b = true;
    }
    void method2(int * i, bool * b) {
        *i = 2; *b = false;
    }
};

class A {
    IApi* m_pApi;
    void M(void (IApi::*)(int*, bool*));
public:
    A() : m_pApi(new IApi()) {}
    void DoThings();
};

void A::M(void (IApi::*mptr)(int*, bool*)) {
    int i;
    bool b;    
    (m_pApi->*mptr)( &i, &b );    
    cout << i << ' ' << b << endl;
}

void A::DoThings(){
    M(&IApi::method1);
    M(&IApi::method2);
}

int main() {
    A a;
    a.DoThings();
}

+1: You beat me to it, I was doing the exact same thing at ideone :P — dario_ramos, May 30 '14 at 20:13

How can I avoid this code duplication?

1 Answers1