Home > Back-end >  Reducing duplicate code for function implementation
Reducing duplicate code for function implementation

Time:12-16

Unfortunately, I get stuck with a problem with duplicated source code. Here is a small example to illustrate my problem:

class cPlayer
{
public:

    struct Properties
    {
        std::vector<cStreet*>    Streets;
        std::vector<cHouse*>     Houses;
        std::vector<cComputer*>  Computers;
        std::vector<cBook*>      Book;
    };

    cPlayer(std::string name) : m_name{name}{};
    ~cPlayer(){};
    std::string         m_name{};
    Properties          m_Properties;
    
    // function overloaded
    void buy(cStreet& Street);
    void buy(cHouse& House);
    void buy(cComputer& Computer);
    void buy(cBook& Book);
};

void cPlayer::buy(cStreet& Street)
{
    std::cout << m_name.c_str() << " : Do you want buy this Street?" << std::endl;
    //Todo: Decision (here yes)
    m_Properties.Streets.push_back(&Street);
};

void cPlayer::buy(cHouse& House)
{
    std::cout << m_name.c_str() << " : Do you want buy this House?" << std::endl;
    //Todo: Decision (here yes)
    m_Properties.Houses.push_back(&House);
};

void cPlayer::buy(cComputer& PC)
{
    std::cout << m_name.c_str() << " : Do you want buy this PC?" << std::endl;
    //Todo: Decision (here yes)
    m_Properties.Computers.push_back(&PC);
};

void cPlayer::buy(cBook& Book)
{
    std::cout << m_name.c_str() << " : Do you want buy this Book?" << std::endl;
    //Todo: Decision (here yes)
    m_Properties.Book.push_back(&Book);
};

So the 4 member functions buy() actually all have the same logic. However, an individual text is output and the individual std::vector is always used. It would of course be much more elegant to implement the function only once. But how? I had only thought of templates, but how can I switch the correct vector() to save the property?

Question after question. Would be great if I could get food for thought, as such a "problem" often appears in my source code.

THANK YOU!

CodePudding user response:

If you like you can refactor it to

void cPlayer::buy(cStreet& Street)
{
     buy_impl(Street,m_Properties.Streets,"some text");
}

and similar for the others, with

template <typename T,typename U>
void buy_impl(const T& t, U& prop,const std::string& message) {
    std::cout << message;
    U.push_back(&t);
}

Though, not sure if added complexity is worth the little savings. Your methods already contain not much more than what is different between them.

CodePudding user response:

It is really not code duplication, as the code is slightly different for each type of property. It is similar, but not identical.

You could move some of the code to a separate helper function and get the buy code down to

void cPlayer::buy(cStreet& Street)
{
    if (ConfirmPurchase("Street"))
       m_Properties.Streets.push_back(&Street);
};

But that's about it, IMO. Don't make the code extra complicated just to reduce duplication.

CodePudding user response:

** A ** solution here is to use inheritance. e.g.

#include <string>
#include <vector>
#include <iostream>

class Property
{
public:
    virtual char const* Name() const = 0;
};

class Street : public Property
{
private:
    static constexpr auto name {"street"};
public:
    char const* Name() const override
    {
        return name;
    }
};


//... etc

class Player
{
    std::vector<Property const*> properties;
public:
    void buy(Property const& property) {
        std::cout << /*...*/ " : Do you want to buy this "
            << property.Name()
            << "?\n";
        // if (yes)
        properties.push_back(&property);
    }
};

int main() {
    Player me {};
    const Street street {};

    me.buy(street);
}
  •  Tags:  
  • c
  • Related