[FrozenSnake]

I have problem with a application I have been working on, it is an attempt to learn how to use decorators.
Feedback I got was that I had one or more memory leaks. So I wonder if I have understood this correctly.

Here is some code, all flavors have the same code only the names are different, flavours are BlackCurrant, Strawberry and Wine.

#ifndef BOTTLE_H
#define BOTTLE_H

#include <string>

// Has no .cpp file!
class Bottle
{
public:
	virtual std::string getLiquid() = 0;
};

#endif

#ifndef WINE_H
#define WINE_H

#include "bottle.h"

class Wine : public Bottle
{
public:
	Wine(Bottle *bottle, int count);
	virtual std::string getLiquid();
private:
	std::string m_Liquid;
	Bottle *m_Bottle;
};

#endif

#ifndef WATER_H
#define WATER_H

#include "bottle.h"

class Water : public Bottle
{
public:
	Water();
	virtual std::string getLiquid();
private:
	std::string m_Liquid;
};

#endif

#include "wine.h"

Wine::~Wine()
{
}

Wine::Wine(Bottle *bottle, int count)
{
	m_Bottle = bottle;
	if(count > 0)
		m_Liquid = ", white wine";
	else
		m_Liquid = " White wine";
}

std::string Wine::getLiquid()
{
	return m_Bottle->getLiquid() + m_Liquid;
}

#include "bottle.h"
#include "water.h"
#include "strawberry.h"
#include "blackcurrant.h"
#include "wine.h"
#include <iostream>

using namespace std;

int main()
{
	Bottle *bottle;
	bottle = new Water;
	int x = 0, count = 0;
	while(x != 5)
	{
		cout << "Choose your flavor:\n1.Strawberry\n2.Blackcurrant\n3.Plain\n4. Wine\nOption: ";
		cin >> x;

		switch(x)
		{
		case 1:
			bottle = new Strawberry(bottle, count);
			cout << bottle->getLiquid() + "!\n\n";
			count++;
			break;
		case 2:
			bottle = new Blackcurrant(bottle, count);
			cout << bottle->getLiquid() + "!\n\n";
			count++;
			break;
		case 3:
			delete bottle; // To "clear" it.
			count = 0;
			bottle = new Water;
			cout << bottle->getLiquid() + "has no flavour!\n";
			break;
		case 4:
			bottle = new Wine(bottle, count);
			cout << bottle->getLiquid() + "!\n\n";
			count++;
			break;
		default:
			cout << "I do not think you wanna drink this!\n\n";
			break;
		}
	}
	//bottle = new Water;
	delete bottle; // I added this since I only use "new" in this part of the code.
	cin.ignore();
	cin.get();
	return 0;
}

Do I use delete correctly here? Or have I forgot it somewhere?
I think I have fixed it but I can't be sure, so would anyone be kind enough to help me?
I might have forgot something, I didn't wanna flood you with code that was identical but if something is
unclear I can add the rest of the code.

[Skydiver]

Yes, you do have leaks. Try drawing out the object instances and pointers on a piece of paper or white board. Remember that C++ could careless if it has a pointer as a member variable in a class, and the destructor code is hit. If you want objects that pointers are pointing to to be deallocated, consider using unique_ptr<> or shared_ptr<>.

[FrozenSnake]

Should I add a virtual destructor and delete inside these?
I though I only should use delete when I use new. Is there something I have missed?
Isn't unique_ptr<> and shared_ptr<> c++11 stuff? My school hasn't started to use c++11 yet so I try to avoid c++11.

[Skydiver]

Yes. Declare a virtual desctructor in the base class, and do the appropriate deletes in the derived classes.

This post has been edited by Skydiver: 17 March 2013 - 09:31 AM

[FrozenSnake]

Thanks for the help!

[baavgai]

Why exactly do some of your child objects take pointers to an object of the parent type?

Why are you making instance of these classes at all? Is getLiquid() supposed to make any changes to the state of the class? If not, make it const. Then, reasonably, do something like:

void showBottle(const Bottle &bottle, const string &post) { cout << bottle.getLiquid() << post; }
void showBottle(const Bottle &bottle) { showBottle("!\n\n"); }

Hope this helps.

[CTphpnwb]

In line 13 you have:

bottle = new Water;

Why? You never use it.

[Skydiver]

baavgai, on 17 March 2013 - 12:43 PM, said:

Why exactly do some of your child objects take pointers to an object of the parent type?

Why are you making instance of these classes at all? Is getLiquid() supposed to make any changes to the state of the class? If not, make it const. Then, reasonably, do something like:

void showBottle(const Bottle &bottle, const string &post) { cout << bottle.getLiquid() << post; }
void showBottle(const Bottle &bottle) { showBottle("!\n\n"); }

Hope this helps.

As he said in the original post, he's trying to experiment with the decorator pattern. With the decorator patter, each decorator holds on to the object that it is decorating.

CTphpnwb, on 17 March 2013 - 12:57 PM, said:

In line 13 you have:

bottle = new Water;

Why? You never use it.

But he is using it. With the decorator pattern, you start off with some basic object and add decorations to it. Depending on which switch case is selected, a new decoration is added.

[baavgai]

Composition? Hmmm... Sounds fun.

Some test code and the basic interface:

#include <iostream>

struct Bottle {
	virtual std::string getLiquid() const = 0;
	virtual int getCount() const = 0;
};
enum BottleType { WATER, WINE };
const Bottle &getBottle(Bottle *&, BottleType, int);
std::ostream &operator<<(std::ostream &, const Bottle &);

using namespace std;

int main() {
	Bottle *b;
	cout << getBottle(b,WATER,1) << endl;
	cout << getBottle(b,WATER,2) << endl;
	cout << getBottle(b,WINE,1) << endl;
	cout << getBottle(b,WINE,2) << endl;
	
	delete b;
	return 0;
}

std::ostream &operator<<(std::ostream &out, const Bottle &B)/> {
	out << b.getCount() 
		<< ' ' << ((b.getCount()==1)?"bottle":"bottles")
		<< " of " << b.getLiquid();
	return out;
}

Right, let me do inheritance first.

class BottleImpl : public Bottle {
public:
	BottleImpl(int c) : count(c) { }
	virtual std::string getLiquid() const { return "Empty Bottle"; }
	int getCount() const { return count; }
private:
	int count;
};


struct WineInherit : public BottleImpl {
public:
	WineInherit(int count) : BottleImpl(count) { }
	std::string getLiquid() const { return "White Wine"; }
};


struct WaterInherit : public BottleImpl {
public:
	WaterInherit(int count) : BottleImpl(count) { }
	std::string getLiquid() const { return "Clear Water"; }
};


const Bottle &getBottle(Bottle *&b, BottleType bt, int count) {
	if (b!=NULL) { delete b; }
	if (bt==WATER) {
		b = new WaterInherit(count);
	} else {
		b = new WineInherit(count);
	}
	return *b;
}

Now, composition:

// still want some kind of concrete class for bottle
class BottleImpl : public Bottle {
public:
	BottleImpl(int c) : count(c) { }
	virtual std::string getLiquid() const { return "Empty Bottle"; }
	int getCount() const { return count; }
private:
	int count;
};

// we don't need this one, but makes building easier
class BottleComposition : public Bottle {
private:
	Bottle *bottle;
public:
	BottleComposition(Bottle *B)/> : bottle(B)/> { }
	~BottleComposition() { delete bottle; }
	// so, from our composition we're getting the very exciting
	// getCount
	int getCount() const { bottle->getCount(); }
};

// and now, some implementations
class WineComposition : public BottleComposition {
public:
	WineComposition(Bottle *B)/> : BottleComposition(B)/> { }
	std::string getLiquid() const { return "White Wine"; }
};

class WaterComposition : public BottleComposition {
public:
	WaterComposition(Bottle *B)/> : BottleComposition(B)/> { }
	std::string getLiquid() const { return "Clear Water"; }
};

const Bottle &getBottle(Bottle *&b, BottleType bt, int count) {
	if (b!=NULL) { delete b; }
	if (bt==WATER) {
		b = new WaterComposition(new BottleImpl(count));
	} else {
		b = new WineComposition(new BottleImpl(count));
	}
	return *b;
}

Ok, honestly, that wasn't all the exciting. Like Object Oriented Programming in general, the benefit doesn't really become apparent until things get big and ugly.

Composition will allow you do avoid nasty C++ multiple inheritance. Curiously, composition partially came out of most other languages implementing on single inheritances, based on C++ nightmares. Java loves it. I'm sure anyone cares that much in C++.

Note, in all that mess, there is an example of how to avoid the memory leak the OP is having...

This post has been edited by baavgai: 18 March 2013 - 06:25 AM
Reason for edit:: tag foo fail

[Skydiver]

The Decorator Pattern: http://en.wikipedia....corator_pattern

The Composite Pattern: http://en.wikipedia....mposite_pattern

Although the class structures look similar, the motivations for the patterns are different.