0

In ref, they have this line of code

Widget *aWidget = new BorderDecorator(new BorderDecorator(new ScrollDecorator
(new TextField(80, 24))));

Two questions:

Say, I want to explicitly delete the objects created with new. How do you do that?

BTW, If I just add 

delete aWidget;

I get warning: deleting object of abstract class type 'Widget' which has non-virtual destructor will cause undefined behaviour [-Wdelete-non-virtual-dtor]

Second related question:

How do you write this nested news with unique_ptr

Thanks

beginner 101
  • 163
  • 5
  • 3
    @tadman: You mean `Widget` needs a virtual destructor. – Fred Larson Apr 19 '17 at 19:14
  • Since you tagged this [tag:raii], it seems that you know that the solution is RAII and have at least heard of it . . . ? – geometrian Apr 19 '17 at 19:15
  • 2
    Time to learn about `std::shared_ptr` etc... – Ed Heal Apr 19 '17 at 19:16
  • Do you have control over the implementations of `Widget`, `BorderDecorator`, `ScrollDecorator`, and `TextField`? – Fred Larson Apr 19 '17 at 19:17
  • 2
    Why do all these constructors take pointers?! – David Schwartz Apr 19 '17 at 19:19
  • @David: The decorator pattern is built that way. See the reference: https://sourcemaking.com/design_patterns/decorator/cpp/2 – beginner 101 Apr 19 '17 at 19:22
  • 2
    This seems like a truly awful pattern to me because it's not clear who owns what. – David Schwartz Apr 19 '17 at 19:25
  • 2
    It probably got borged from Java and hasn't found a good implementation yet. And this is a mistake I would not want to see in a textbook: `class Widget { public: virtual void draw() = 0; };` Compiler is 100% on the nose to complain. – user4581301 Apr 19 '17 at 19:28
  • @DavidSchwartz Well, the implementation that was linked is awful. The general idea of the pattern is probably sane. – simon Apr 19 '17 at 19:29
  • My advice then would simply be to ignore that page. It definitely looks like it was not actually designed to be C++ code but was Java code that was minimally changed with the intent of illustrating the pattern, not showing how to actually implement the pattern in C++. – David Schwartz Apr 19 '17 at 19:39

2 Answers2

3

The example you're following glosses over all the memory management. It's likely to lead to bad habits.

To properly manage memory, you could simply avoid dynamic allocations:

TextField textField(80, 24);
ScrollDecorator scrollDecorator(&textField);
BorderDecorator bd1(&scrollDecorator);
BorderDecorator bd2(&bd1);
Widget *aWidget = &bd2;

No news is good news!

And Widget really, really should have a virtual destructor.

Fred Larson
  • 60,987
  • 18
  • 112
  • 174
  • 1
    "No `new`s is good news!". Amen. Here is a few slides that also explains this: http://klmr.me/slides/modern-cpp.pdf – simon Apr 19 '17 at 19:31
  • 1
    @gurka: As far as I'm aware, I just coined that phrase myself (in this context, meaning C++ `new`). I think it's going to be my new motto. – Fred Larson Apr 19 '17 at 19:35
1

Your problem isn't that you're deleting the object, your problem is that you're deleting a potentially polymorphic object which doesn't have a virtual destructor, which leaves the possibility that the most derived class won't be properly cleaned up.

See here for more information.

Community
  • 1
  • 1
Robert Allan Hennigan Leahy
  • 6,478
  • 28
  • 45