How do I optimise c++ legacy boiler plate code?

Question

I am a Java user trying my hand at c++.

In the below legacy code, I have multiple derived classes.

The contents of the map in each derived class are similar differing by just a few.

The function pr() is repetitive.

I would like to optimize such piece of code.

Do let me know ideas/suggestions?

Thanks

#include <iostream>
#include <map>
#include <string>
#include <functional>
using namespace std;
#include <memory>


class X {
public:
    virtual void pr ()=0;

};



class Y: public X {
        std::map<string,int> fm{ {"mem1", 33},
                                 {"mem2", 44},
                                 {"YYY", 999}};

public:
    virtual void pr () {
        for(auto const &[k,v] : fm)
        {
             std::cout<<k <<"  :  "<<v <<std::endl;
        }
    }
};

class Z: public X {
    sstd::map<string,int> fm{ {"mem1", 33},
                              {"mem2", 44},
                              {"ZZZ", 777}};

public:
    virtual void pr () {
        for(auto const &[k,v] : fm)
        {
            std::cout<<k <<"  :  "<<v <<std::endl;
        }
    }
};

void fn(std::unique_ptr<X> x)
{
    x->pr();  
}

int main()
{
   fn(std::make_unique<Y>());
}

TBH, the one solution that strikes would be the same as in Java: Make `X::pr` non-abstract, move the definition from derived classes to `X`, move `fm` to X as well and either initialize it with default value and modify in derived classes constructor or create `X` constructor that receives ready-made map. — Yksisarvinen, Sep 28 '20 at 11:07
"optimize" is extremely vague. I suppose you want to reduce repetition of similar code, is that right? — 463035818_is_not_an_ai, Sep 28 '20 at 11:07

score 2 · Answer 1 · answered Sep 28 '20 at 11:18

I would drop the derived classes, you only have a difference in construction.

#include <map>
#include <string>
#include <utility>

class X {
    std::map<string,int> fm;
public:
    X(std::map<string,int> fm): fm(std::move(fm)) {}
    void pr () {
        for(auto const &[k,v] : fm)
        {
             std::cout<<k <<"  :  "<<v <<std::endl;
        }
    }
};

X Y(){ return X({ {"mem1", 33}, {"mem2", 44}, {"YYY", 999}}); }
X Z(){ return X({ {"mem1", 33}, {"mem2", 44}, {"ZZZ", 777}}); }

int main()
{
    Y().pr();
}

Muhammet Ali Asan · Accepted Answer · 2020-09-28T12:08:35.093

This is not called optimization. This is called refactoring code to make use of inheritance. Apparently, in your code, repetition is in data member fm and pr() function definition.

If your both classes have them in common, you can just move pr function definition and data member fm to base class so that you do not have to rewrite. Then you can initialize value of fm, which is only difference between two classes. =0 makes a virtual function pure, which should be overriden by all inheriting classes. In this case, they do not override, instead they reuse pr function, so pure function is not needed.

#include <iostream>
#include <map>
#include <string>
#include <functional>
#include <memory>
using namespace std;


class X {
public:
    virtual void pr (){
         for(const auto item : fm)
        {
             std::cout<<item.first <<"  :  "<<item.second <<std::endl;
        }
    };
    protected:
    std::map<string,int> fm;
};



class Y: public X 
{
    public:
    Y(){
      fm =   { {"mem1", 33},
             {"mem2", 44},
             {"YYY", 999}};
    }
};

class Z: public X 
{
    public:
    Z(){
      fm =   { {"mem1", 33},
                              {"mem2", 44},
                              {"ZZZ", 777}};
    }
};

void fn(std::unique_ptr<X> x)
{
    x->pr();  
}

int main()
{
   fn(std::make_unique<Y>());
      fn(std::make_unique<Z>());

 return 0;
}

you should initialize the map in the constructors member initializer list — 463035818_is_not_an_ai, Sep 28 '20 at 12:24

How do I optimise c++ legacy boiler plate code?

2 Answers2