I have a hierarchy of Money , Dollar and Franc classes that simulate money. It is necessary in the style of OOP to implement the following:

  • Multiply money by number
  • Money comparison
  • Test code

I wrote this:

 #ifndef MONEY_H #define MONEY_H #include <memory> using std::shared_ptr; using std::make_shared; class Dollar; class Money { public: static shared_ptr<Money> dollar(int amount); virtual shared_ptr<Money> times(int value) const = 0; virtual bool equals(shared_ptr<Money> money) const = 0; protected: int amount; }; class Dollar : public Money { public: Dollar(int amount); shared_ptr<Money> times(int value) const override; bool operator==(const Dollar& dollar) const; bool equals(shared_ptr<Money> dollar) const override; }; class Franc : public Money { public: Franc(int amount); shared_ptr<Money> times(int value) const override; bool operator==(const Franc& franc) const; bool equals(shared_ptr<Money> dollar) const override; }; #endif #include "money.h" #include <gmock\gmock.h> shared_ptr<Money> Money::dollar(int amount) { return make_shared<Dollar>(amount); } Dollar::Dollar(int amount) { this->amount = amount; } shared_ptr<Money> Dollar::times(int value) const { return make_shared<Dollar>(amount * value); } bool Dollar::operator==(const Dollar& dollar) const { return amount == dollar.amount; } bool Dollar::equals(shared_ptr<Money> dollar) const { if (dynamic_cast<shared_ptr<Dollar>>(dollar)) { return true; // TODO } return false; } Franc::Franc(int amount) { this->amount = amount; } shared_ptr<Money> Franc::times(int value) const { return make_shared<Franc>(amount * value); } bool Franc::operator==(const Franc& franc) const { return amount == franc.amount; } using ::testing::Eq; TEST(Money, TestDollarMultiplication) { shared_ptr<Money> five = Money::dollar(5); ASSERT_TRUE(five->times(2) == Dollar(10)); ASSERT_TRUE(five->times(3) == Dollar(15)); } TEST(Money, TestFrancMultiplication) { const Franc five(5); ASSERT_TRUE(five.times(2) == Franc(10)); ASSERT_TRUE(five.times(3) == Franc(15)); }

There were problems due to tests. It is necessary to compare money with each other. Since I work with pointers due to polymorphism, this brings additional inconvenience.

It turned out that after the transition from objects to pointers, the test stopped compiling:

 TEST(Money, TestDollarMultiplication) { shared_ptr<Money> five = Money::dollar(5); ASSERT_TRUE(five->times(2) == Dollar(10)); ASSERT_TRUE(five->times(3) == Dollar(15)); }

At first I used the comparison operator, but now I needed the equals function for comparison by shared points.

 bool Dollar::equals(shared_ptr<Money> dollar) const { if (dynamic_cast<shared_ptr<Dollar>>(dollar)) { return true; // TODO } return false; }

You need to make sure that pointers to objects of the same type are compared. I use RTTI for this, but using this dynamic_cast results in an error.

How to do right?

  • see Casting C ++ Smart Pointer Types . Another thing is that neither the hierarchy of polymorphic classes, nor shared_ptr , nor dynamic_cast are particularly needed for solving this task ... - VTT
  • Why not need? How then to use polymorphism and hide low-level classes behind the interface Money? - typemoon
  • Yes, and polymorphism is also not particularly needed. It is enough to make one class with two fields - quantity and currency. - VTT
  • From what you have written so far, it is not at all clear what you need to do. Comparing polymorphic objects, such as currencies (that is, when different types are actually comparable) is a classic problem on double dispatch. However, you suddenly have a type matching check done. Is it really necessary to do this? - AnT

1 answer 1

Implementing a comparison via dynamic_cast is a bad idea. In the case when you can only compare money of one currency - it is still tolerable, but it will be problematic to compare different currencies. It is better to enter some base currency (not the heir of money) into which you can convert any other using the appropriate virtual method, and compare these base currencies.

In addition, all use cases of shared_ptr in your code do not make sense. In most cases, you can use unique_ptr, in some places you can create objects on the stack.

Passing objects to a function via shared_ptr is also a bad idea, unless the pointer is saved as a side effect inside the function. In a function similar to equals, it is better to pass arguments over a constant reference to the base class, and already when called to bring the pointer to the link.

The times function in theory should not create a new object, just modify the old one. But if you want, return again unique_ptr, not shared_ptr.

Well, directly on the PLO: the base class usually does not need to know anything about its heirs, otherwise its meaning is lost.