2

Given following problem:

class Instrument {
};

class Guitar : public Instrument {
  public:
    void doGuitar() const;
};

class Piano : public Instrument {
  public:
    void doPiano() const;
};

I got a list of pointers to Instrument

list<shared_ptr<Instrument>> instruments;

in which i add instruments via (for example)

Guitar myGuitar;
instruments.push_back(make_shared<Guitar>(myGuitar));

Now, I want to iterate over the list instruments and call doPiano() iff the current instrument is a piano and doGuitar() iff it is a guitar. These two functions differ a lot and thus, cannot be made abstract in class Instrument.

The problem is that C++ won't be able to identify the type of the Instrument by runtime, isn't it (due to single dispatch)? How can I achieve that it calls the piano- or the guitar-function depending on the current type the iterator is pointing at.

I would be happy if I could implement sth. working like this pseudo-code:

list<shared_ptr<Instrument>>::const_iterator it;
if ("current type == Guitar")
  (*it)->doGuitar();
else if ("current type == Piano")
  (*it)->doPiano();

Result

Actually, I ran into several problems with my approach. I did much refactoring using this post: How does one downcast a std::shared_ptr? . Thanks to all for your help :)

Community
  • 1
  • 1
Kapa11
  • 311
  • 2
  • 18
  • 4
    Why not simply have a virtual `do` function that does the right thing? – Some programmer dude Dec 12 '16 at 09:25
  • have a look at `std::dynamic_pointer_cast`. Then rip up your design and start again. Polymorphism is only suitable when all derived classes can reasonably share the same interface. – Richard Hodges Dec 12 '16 at 09:27
  • Actually, my example is just a small piece of my "real" implementation. There are several pure virtual functions in `Instrument`. I should have mentioned it. – Kapa11 Dec 12 '16 at 09:33
  • Since you mention multiple dispatch you might want to take a look at the [visitor pattern](https://en.wikipedia.org/wiki/Visitor_pattern#C.2B.2B_example). – G.M. Dec 12 '16 at 09:34
  • 2
    "These two functions differ a lot and thus, cannot be made abstract in class Instrument." Show real code. As it stands now, do_piano and do_guitar are exactly the same and should be made a common pure virtual function in Instrument. You need a compelling argument to the contrary. – n. m. could be an AI Dec 12 '16 at 09:41

4 Answers4

3

The design can probably be improved to eliminate this problem, but working within the existing design you can add a virtual member function Instrument::play_it that takes a Player as polymorphic argument. In Player have two functions play_guitar (taking guitar argument) and play_piano (taking piano argument). In guitar class override play_it to call Player::play_guitar with self as argument. In piano class override play_it to call Player::play_piano with self as argument. Look ma no casts.

This isn't exactly multiple dispatch, it's known as the visitor pattern. However it's perhaps best to not focus too much on that, lest you start to name things visitor or such non-descriptive folly.

Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
0

A double dispatch works like this (pseudocode, important but trivial stuff omitted):

struct InstrumentVisitor{
   // knows all instruments
   virtual void doGuitar(Guitar*) = 0;
   virtual void doPiano(Piano*) = 0;
};

class Instrument {
   virtual void doInstrument(InstrumentVisitor*) = 0;
   ...
 };

class Piano : public Instrument {
    void doInstrument (InstrumentVisitor* v) {
       v->doPiano(this);
};

class Guitar : public Instrument {
    void doInstrument (InstrumentVisitor* v) {
       v->doGuitar(this);
};

Now we can devise concrete visitors.

struct Player : InstrumentVisitor {
  // does vastly different things for guitar and piano
   void doGuitar (Guitar* g) {
      g->Strum(pick, A6);
   }
   void doPiano (Piano* p) {
      p->Scale (Am, ascending);
};
n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243
  • @Cheersandhth.-Alf For not having to write `*this` which can be really confusing at times. But it''s a matter of taste. Use references or pointers, I won't object. – n. m. could be an AI Dec 12 '16 at 10:33
0

Type erasure is another option:

std::vector<std::function<void()>> playInstrument;
playInstrument.emplace_back([g = Guitar{}]() { return g.doGuitar(); });
playInstrument.emplace_back([p = Piano{} ]() { return p.doPiano();  });

playInstrument[0]();

For this you even don't need a common base class.

davidhigh
  • 14,652
  • 2
  • 44
  • 75
-2

One way to identify classes at runtime is to use dynamic_cast. But to use that, you need to have atleast one virtual method in your class. An empty virtual method can be added to the instrument class for that purpose.

class Instrument {
  private:
    virtual void emptyMethod_doNotCall() {} // Add this method.
};

class Guitar : public Instrument {
  public:
    void doGuitar() const;
};

class Piano : public Instrument {
  public:
    void doPiano() const;
};

The object type can be checked by doing a dynamic_cast to the target class pointer. dynamic_cast returns NULL if the object cannot be cast to the desired target class.

list<shared_ptr<Instrument>>::const_iterator it;
if (dynamic_cast<Guitar*>(it) != NULL)
  (*it)->doGuitar();
else if (dynamic_cast<Piano*>(it) != NULL)
  (*it)->doPiano();
Chethan Ravindranath
  • 2,001
  • 2
  • 16
  • 28
  • 1
    each time a new instrument is added, you need to go over all of the code, add manually the case of this new instrument... why bother with a class hierarchy then? – UmNyobe Dec 12 '16 at 09:35
  • Why not just make the destructor `virtual`? After all, since you're storing by base pointer it going to need to be. – Sean Dec 12 '16 at 09:41
  • @UmNyobe: The OP says "These two functions differ a lot and thus, cannot be made abstract in class Instrument". That's the reason I suggested doing a dynamic cast. – Chethan Ravindranath Dec 12 '16 at 09:44
  • Dynamic cast in a pattern like this is your last resort when you need to save a failed design by 09:00 today, and it's 7:55 and you have had 3 hours of sleep during the last week. – n. m. could be an AI Dec 12 '16 at 10:22
  • Trying this method, leads to following compiler error: `cannot dynamic_cast 'it' (of type 'struct _list_const_iterator>'') to type 'class Guitar*' (source is not a pointer)`. – Kapa11 Dec 12 '16 at 11:24