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