2

EDIT: note I want a static compile time method, when I know exactly what needs to go where at compile time.

I often find myself having multiple functions which follow the same pattern, but only a few parts change. Often I opt for the Template Method Pattern, however class based approaches (especially in c++) seem very cumbersome. I'm often forced to create actual objects when really all I'm trying to do is change one line for another that can't be prepended or appended to the routine I'm writing, where ideally I'd want to simply generate a function given a pattern at compile time. I even need to create private/protected classes where the procedure only makes sense in the class I'm using. This all seems like a huge waste.

I'd like a static approach to this, where I know exactly which function I would like to use in each situation, but I don't want duplicated code between each function where only a small amount changes. Is it possible in something like C++ with out resorting to creating custom classes for each case this happens? There are "solutions" like creating several custom functions for each step, but with out TMP these functions become coupled to a high degree and require way more parameters to be passed around than normally needed, and still never really seem to get rid of code duplication.

EDIT most recent example I came across since I was asked for examples...

class X{
    A m_class_member_1;
    B m_class_member_2;
    C m_class_member_3;
    D m_class_member_4;
    E m_class_member_5;
    ...
    void duplicateProcedure_1(){
        auto some_value = m_class_member_1.function();
        if(valuePassesTest(some_value)){
            auto some_other_value = m_class_member_2.function(some_value);
            m_class_member_3.function(some_other_value);
            m_class_member_4.function(some_other_value);
            // changed procedure
            m_class_member_5.function_1(some_other_value);
        }
    }

    void duplicateProcedure_2(){
        auto some_value = m_class_member_1.function();
        if(valuePassesTest(some_value)){
            auto some_other_value = m_class_member_2.function(some_value);
            m_class_member_3.function(some_other_value);
            m_class_member_4.function(some_other_value);
            //changed procedure
            m_class_member_5.function_2(some_other_value);
        }
    }
}

the last line in each is the only thing that needs to be changed, everything else needs to be the same, but they also depend on variables created with in the procedure, as well as conditions outside the scope of their block.

  • Posting some illustrative code will be nice. – R Sahu Jun 9 '17 at 18:50
  • @RSahu added code to illustrate the most recent example I've come accross, but I've had static TMP problems a lot, and I don't want to pigeon-hole this question to this one example, I would like a general solution for these types of situations. – opa Jun 9 '17 at 19:12
  • what's wrong with extracting all the unchanged lines in the if body to a new method called from both places? – Timothy Truckle Jun 9 '17 at 19:15
  • @TimothyTruckle the changed line lies in the scope of a block whose outer components are unchanged. Unless you have another way, to use this you would be forced to create a function that accepts a function pointer, which I certainly don't see as an adequate solution when I know at compile time what should go where. Even if there was no block scope, if m_class_member needed more variables inside the unchanged part, or was dependent on another changing variable, this would cause a glut of parameters needed to be passed out of the extracted functions, again, not an acceptable solution. – opa Jun 9 '17 at 19:22
  • "the changed line lies in the scope of a block whose outer components are unchanged, to use this you would be forced to create a function that accepts a function pointer" - I meant a function that returns some_other_value replacing all but the last line like if(valuePassesTest(some_value)){m_class_member_5.function_2(this.new_function_replacing_previous_lines(some_value));} – Timothy Truckle Jun 9 '17 at 19:26
2

You're right to be cautious on abusing of abstractions. This is well described in this article.

I can see 3 alternatives to that:

  • Composition: you keep the 2 duplicated procedures and extract just the common parts to some private function. Easy, readable, there may still have some duplicated parts but it's kept reasonable. The down side is that you may come to create many tiny private functions for that.
  • Anonymous functions / lambdas; except if you have good reason to avoid it (is it why you mention "imperative" oo language?), it solves exactly this kind of issue; but could lead to less readable code eventually.
  • And finally sometimes we focus too much on removing duplicated parts, while we shouldn't. Duplication is not necessarily bad (as explained in the link above). I would say: try to reduce duplicated code up to a point. Compose as long as it's beneficial without doubt, but there's no reason to be ashamed when leaving some duplicated code.
  • Composition is certainly acceptable, I just ideally would like to extract all the parts, but the article you link is really interesting. I may have to compromise on getting rid of duplication. I mentioned imperative language because I would never have this problem in Haskell, I would just compose the right function. While a function pointer based solution could work, I see it as a needless waste of resources, it wouldn't be a zero cost abstraction, (and my function signature could change depending on the function). I like this answer the most right now though. – opa Jun 9 '17 at 19:42
1

I don't know if C++ allows you to pass bound methods around as callbacks, but if it does, the easiest solution is probably something like this:

void consolidatedProcedure(SOME_CALLBACK_TYPE callback){
    auto some_value = m_class_member_1.function();
    if(valuePassesTest(some_value)){
        auto some_other_value = m_class_member_2.function(some_value);
        m_class_member_3.function(some_other_value);
        m_class_member_4.function(some_other_value);
        // changed procedure
        callback(some_other_value);
    }
}

void duplicateProcedure_1(){
    consolidatedProcedure(m_class_member_5.function_1);
}

void duplicateProcedure_2(){
    consolidatedProcedure(m_class_member_5.function_2);
}

You say that you're looking for a general solution for these types of situations. The problem is, the more general the question is, the more general the answer is. The general solution is to pass around objects and/or functions as parameters, or to use templates. If you want a more specific answer, you'll probably need a more specific question.

  • I should have qualified the question, yes C++ has bound methods, and using a callback is an alternative, however I'm only interested in static implementations, AFAIK since the compiler doesn't know what function you want to put in, it can't inline the callback, and at the very least it won't know which function I want to use at compile time. – opa Jun 9 '17 at 19:35
  • It sounds like you're saying that you don't want to use callbacks or virtual methods for performance reasons? Have you tried it out and determined that the callbacks are causing performance issues? – Tanner Swett Jun 9 '17 at 19:42
  • It violates zero cost abstraction principle, I know how to do this with out hurting performance but abusing duplication abstractions would lead to an unnecessary performance loss, with possible loss in code readability even if there may be less bugs as a result. The fact that the non abstracted solution is both easier, easier to read, and has no cost calls into question these other types of abstractions. You are only supposed apply the "if it isn't the largest cause of performance issues don't fix it" when the alternative is harder to read and more difficult to implement. – opa Jun 9 '17 at 19:51
  • see R Sahu's solution for using templates to avoid the whole wasted performance via templating on any function like object. No need to deal with messy predefined signatures and no performance loss, however the fact that you removed where the procedure went still makes it less readable (which he also solves by taking a judgment approach on duplicating some when you don't use it much, and using the template approach when it is used a lot) – opa Jun 9 '17 at 20:07
1

You will be able to avoid code duplication by using a common function that takes a lambda function as an argument.

void duplicateProcedure_1(){
   commonProcedure([&m_class_member_5](some_other_value_type some_other_value)
                   {m_class_member_5.function_1(some_other_value);});
    }
}

void duplicateProcedure_2(){
   commonProcedure([&m_class_member_5](some_other_value_type some_other_value)
                   {m_class_member_5.function_2(some_other_value);});
    }
}

private:

template <typename F>
void commonProcedure(F f){
    auto some_value = m_class_member_1.function();
    if(valuePassesTest(some_value)){
        auto some_other_value = m_class_member_2.function(some_value);
        m_class_member_3.function(some_other_value);
        m_class_member_4.function(some_other_value);
        f(some_other_value);
    }
}

However, I understand that this is illustrative code, not necessarily the production code. If the production code is more involved, it's quite likely that you will lose readability. The cost of lost readability might be more than the cost of duplicated lines of code.

If commonProcedure will be used by 5 or more procedures, I would go with the above solution.

If commonProcedure will be used by 3 or fewer procedures, I would go with the duplicated code.

If commonProcedure will be used by exactly 4 procedures, you have weigh the cost and benefits of the approaches for your situation and make a judgement call.

  • Ok, I see, I suppose I would also have to friend this common procedure inside my class X in my example? It never really occurred to me that I should be weighing the options based upon how many functions use the duplicated code, that helps a lot with how I should move forward on these sorts of issues! – opa Jun 9 '17 at 20:01
  • @snb, if the common function can be a member function, that will be best. If not, yes you have to make it a friend of the class. – R Sahu Jun 9 '17 at 20:05
  • Ah I just looked it up, I was under the impression I would be forced to friend it if it was a template in a non template class, it looks like that isn't so – opa Jun 9 '17 at 20:09

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.