overloaded functions skipped on references

Question

The below code attempts to implement basic Decorator Design Pattern using references to refer to the base component class Paragraph

#include <string>
#include <iostream>

using std::cout; using std::endl; using std::string;

// component
class Paragraph{
public:
   Paragraph(const string& text = "") : text_(text) {}
   virtual string getHTML() const { return text_; }
private:
   const string text_;
};

// first decorator
class BoldParagraph : public Paragraph {
public:
   BoldParagraph(const Paragraph& wrapped): wrapped_(wrapped) {}
   string getHTML() const override {
      return "<b>" + wrapped_.getHTML() + "</b>";
   }
private:
   const Paragraph &wrapped_;
};


// second decorator
class ItalicParagraph : public Paragraph {
public:
   ItalicParagraph(const Paragraph& wrapped): wrapped_(wrapped) {}
   string getHTML() const override {
      return "<i>" + wrapped_.getHTML() + "</i>";
   }
private:
   const Paragraph &wrapped_;
};

int main(){
   Paragraph p("Hello, World!");

   BoldParagraph     bp(p); cout <<  bp.getHTML() << endl;
   BoldParagraph   bbp(bp); cout << bbp.getHTML() << endl; 
   ItalicParagraph ibp(bp); cout << ibp.getHTML() << endl;
}

When run, it produces

<b>Hello, World!</b>
<b>Hello, World!</b>
<i><b>Hello, World!</b></i>

rather than

<b>Hello, World!</b>
<b><b>Hello, World!</b></b>
<i><b>Hello, World!</b></i>

That is, the second wrapped function getHML() appears to be skipped if it is for the same subclass BoldParagraph for object bbp, but not if it is for different subclasses ItalicParagraph and BoldParagraph for object ibp. Similar code with pointers rather than references works as expected.

why is that?

This is bizarre. Happens in gcc and VS. The `_wrapped` in `ibp` is the only one with a `BoldParagraph` vtable. — zzxyz, Nov 21 '17 at 19:16
`&bbp.wrapped_` and `&p` are the same memory address. Providing a copy constructor fixes it, but I'm honestly not sure how the default copy constructor could cause this to occur. — zzxyz, Nov 21 '17 at 19:34

zzxyz · Answer 1 · 2017-11-21T20:51:46.873

The default copy constructor is being called. This creates a copy of your original object passed in (bp), rather than a wrapper around bp. You could fix by declaring your own copy constructor, but I don't recommend it:

  BoldParagraph(const BoldParagraph &wrapped) : wrapped_(wrapped) {}

To clarify, the default copy constructor copies the values passed in object, like so:

BoldParagraph(const BoldParagraph &original) : wrapped_(original.wrapped_) {}

And, the more I think about it, the more I think my suggested fix is incorrect. There can/probably will be nasty side-effects. I would recommend fixing by making your constructors take pointers.

Answers here will show possible side-effects of having a copy constructor that does not perform a copy: Copy Constructor in C++ is called when object is returned from a function?

A concrete example using a copy constructor that wraps instead of copies:

void AFunction(BoldParagraph bp)
{
  cout << bp.getHTML() << endl;
}
int main() {
  Paragraph p("Hello, World!");

  BoldParagraph     bp(p); cout << bp.getHTML() << endl;
  BoldParagraph   bbp(bp); cout << bbp.getHTML() << endl;
  AFunction(bbp);

output:

<b>Hello, World!</b>
<b><b>Hello, World!</b></b>
<b><b><b>Hello, World!</b></b></b>

score 1 · Answer 2 · answered Nov 21 '17 at 19:47

The reason is that bbp(bp) with bp being of type BoldParagraph calls the (implicitly defined) copy constructor BoldParagraph::BoldParagraph(const BoldParagraph &)This then performs a member wise copy/assignment, such that wrapper_ gets sliced and will then be of type Paragraph & rather than BoldParagraph &. To overcome this, you'll have to provide an explicit copy constructor for BoldParagraph::BoldParagraph(const BoldParagraph &), which then calls your individual one:

 BoldParagraph(const BoldParagraph& wrapped) : BoldParagraph((Paragraph&)wrapped) {} ;

score 0 · Answer 3 · answered Nov 21 '17 at 20:28

0

Oh, great, thank you much! That was hard to spot. I ended up adding this

 BoldParagraph(const BoldParagraph& wrapped) : 
         BoldParagraph(static_cast<const Paragraph&>(wrapped)) {}

to make the code happy

answered Nov 21 '17 at 20:28

Mikhail

173
6

Here's why you might not want to do that: `void AFunction(BoldParagraph bp){ cout << bp.getHTML() << endl;}` Try calling this with bbp – zzxyz Nov 21 '17 at 20:47
1

eeek, have not thought of that – Mikhail Nov 21 '17 at 20:58

overloaded functions skipped on references

3 Answers3