'Reducing duplicate code for function implementation

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!

c++


Solution 1:[1]

** 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);
}

Solution 2:[2]

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.

Solution 3:[3]

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.

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1
Solution 2 BoP
Solution 3 463035818_is_not_a_number

Related Questions