NVI idiom and patterns such as Decorator and Proxy

C

Christian Hackl

Hi!

I've got a design question related to the combination of the NVI idiom
(non-virtual interfaces, [1]) and popular object-oriented patterns such
as Proxy or Decorator, i.e. those which have the basic idea of deriving
from a base class and delegating to an object of it at the same time.

My problem is that I cannot seem to combine those two techniques in a
flawless way. For a very simple, non real life example (for which I
shall omit smart pointers and std::strings :)), let's say I've got an
abstract base class Printer, from which ConcretePrinter is derived. I'd
express this in C++ as follows:

class Printer
{
//...
public:
virtual ~Printer() {}
void print(char *str);
private: // could also be protected; doesn't matter for my problem
virtual void doPrint(char *str) = 0;
};

class ConcretePrinter : public Printer
{
//...
public:
virtual ~ConcretePrinter() {}
private:
virtual void doPrint(char *str);
};

The point is that Printer::print can do some parameter checking before
delegating to doPrint:

void Printer::print(char *str)
{
if (str)
doPrint(str);
}

The derived class then actually performs the operation:

void ConcretePrinter::doPrint(char *str) { cout << str << endl; }


So far, that's fine. But now let's say I want to use a Decorator to add
some extra output:

class PrinterDecorator : public Printer
{
public:
virtual ~PrinterDecorator() {}
PrinterDecorator(Printer *decorated_printer) :
decorated_printer_(decorated_printer) {}
//...
private:
virtual void doPrint(char *str)
{
cout << "some decoration..." << endl;
decorated_printer_->print(str); // <-- line that bugs me
cout << "some decoration..." << endl;
}
Printer *decorated_printer_;
};

The print() call in this piece of code is what bugs me. I cannot call
decorated_printer_'s doPrint() because it is non-public in this context,
but calling print() means that all parameter checking performed in
Printer::print() is uselessly duplicated, and it would be duplicated
again for all further decorators or proxies I might add. After all, by
the time PrinterDecorator::doPrint() is called, all necessary checking
already took place in Printer::print(). It's like the derived class
telling the base class, "I know you already checked the data, but please
check it again anyway."

Granted, in this stupid example, it's just a pointer check, but think of
more expensive operations such as "do files exist", "can server be
accessed", or "lock for other threads". A program that duplicates such
operations "by design" doesn't strike me as very well designed.

How do I cope with this situation? Is it just an unfortunate fact of
life that NVI and Decorator/Proxy-like patterns don't mix? Or am I
missing something?

In fact, I thought of a possible solution. If I gave Printer an
additional protected "print" method that took a Printer object and that
did *nothing* but delegate to doPrint(), I could call that protected
method from the decorator:

class Printer
{
//...
public:
void print(char *str); // clients of Printer keep calling this one
protected:
void print(Printer *printer, char *str) // <-- new method, to be

// called by derived classes
// if they need to
{
printer->doPrint(str);
}
private:
virtual void doPrint(char *str) = 0;
};

void PrinterDecorator::doPrint(char *str)
{
cout << "some decoration..." << endl;
print(decorated_printer_, str); // <-- now calling the new
// protected method
cout << "some decoration..." << endl;
}


Is this a good idea? It looks a bit confusing even to me although I came
up with it myself :) Furthermore, I realise that nothing keeps authors
of subclasses from calling the public print() method instead of the
protected one. Then again, nothing in C++ keeps them from doing much
more evil things such as overriding non-virtual public base methods in a
NVI class hierarchy, subclassing classes that are documented as "final",
and so on; maybe this is just another situation in which you can hold
the author of the subclass responsible for abusing the base class.

So in conclusion, I am very unsure what to do. Is it wiser to generally
design classes without the NVI idiom, mixing virtuality and
public access by default (which is exactly what I managed to unlearn),
thus sacrificing all its benefits for easier application of useful
design patterns such as Decorator and Proxy?


[1] http://www.gotw.ca/publications/mill18.htm
 
G

Greg Herlihy

I've got a design question related to the combination of the NVI idiom
(non-virtual interfaces, [1]) and popular object-oriented patterns such
as Proxy or Decorator, i.e. those which have the basic idea of deriving
from a base class and delegating to an object of it at the same time.

My problem is that I cannot seem to combine those two techniques in a
flawless way. For a very simple, non real life example (for which I
shall omit smart pointers and std::strings :)), let's say I've got an
abstract base class Printer, from which ConcretePrinter is derived. I'd
express this in C++ as follows:

class Printer
{
//...
public:
virtual ~Printer() {}
void print(char *str);
private: // could also be protected; doesn't matter for my problem
virtual void doPrint(char *str) = 0;

};

class ConcretePrinter : public Printer
{
//...
public:
virtual ~ConcretePrinter() {}
private:
virtual void doPrint(char *str);

};

The point is that Printer::print can do some parameter checking before
delegating to doPrint:

void Printer::print(char *str)
{
if (str)
doPrint(str);

}

The derived class then actually performs the operation:

void ConcretePrinter::doPrint(char *str) { cout << str << endl; }

So far, that's fine. But now let's say I want to use a Decorator to add
some extra output:

class PrinterDecorator : public Printer
{
public:
virtual ~PrinterDecorator() {}
PrinterDecorator(Printer *decorated_printer) :
decorated_printer_(decorated_printer) {}
//...
private:
virtual void doPrint(char *str)
{
cout << "some decoration..." << endl;
decorated_printer_->print(str); // <-- line that bugs me
cout << "some decoration..." << endl;
}
Printer *decorated_printer_;

};

The print() call in this piece of code is what bugs me. I cannot call
decorated_printer_'s doPrint() because it is non-public in this context,
but calling print() means that all parameter checking performed in
Printer::print() is uselessly duplicated, and it would be duplicated
again for all further decorators or proxies I might add. After all, by
the time PrinterDecorator::doPrint() is called, all necessary checking
already took place in Printer::print(). It's like the derived class
telling the base class, "I know you already checked the data, but please
check it again anyway."

Granted, in this stupid example, it's just a pointer check, but think of
more expensive operations such as "do files exist", "can server be
accessed", or "lock for other threads". A program that duplicates such
operations "by design" doesn't strike me as very well designed.

How do I cope with this situation? Is it just an unfortunate fact of
life that NVI and Decorator/Proxy-like patterns don't mix? Or am I
missing something?

The solution is to declare PrinterDecorator a friend of the Printer
class. As a friend of the Printer class, PrinterDecorator's doPrint()
method will be able to call doPrint() on its wrapped Printer object:

class Printer
{
friend class PrinterDecorator;
...
};

virtual void doPrint(char *str)
{
cout << "some decoration..." << endl;
decorated_printer_->doPrint(str); // OK
cout << "some decoration..." << endl;
}

Furthermore, doPrint() should really be declared protected instead of
private - since a derived class may wish to call its base class'
version of doPrint() in the course of executing its own doPrint().
(Note that - even if doPrint() were declared protected instead of
private - the friend declaration would still be needed in order for
PrinterDecorator to be able to call its decorated_printer's doPrint()
method).

Greg
 
G

gpuchtel

My first reaction was to mention 'friendship' as well; however, a base
class shouldn't have to know its going to be 'decorated'. The whole
idea of the 'decorator' pattern is to embellish an existing
implementation without its knowledge.
 
J

James Kanze

I've got a design question related to the combination of the NVI idiom
(non-virtual interfaces, [1]) and popular object-oriented patterns such
as Proxy or Decorator, i.e. those which have the basic idea of deriving
from a base class and delegating to an object of it at the same time.
My problem is that I cannot seem to combine those two techniques in a
flawless way. For a very simple, non real life example (for which I
shall omit smart pointers and std::strings :)), let's say I've got an
abstract base class Printer, from which ConcretePrinter is derived. I'd
express this in C++ as follows:
class Printer
{
//...
public:
virtual ~Printer() {}
void print(char *str);
private: // could also be protected; doesn't matter for my problem
virtual void doPrint(char *str) = 0;
};
class ConcretePrinter : public Printer
{
//...
public:
virtual ~ConcretePrinter() {}
private:
virtual void doPrint(char *str);
};
The point is that Printer::print can do some parameter
checking before delegating to doPrint:
void Printer::print(char *str)
{
if (str)
doPrint(str);
}
The derived class then actually performs the operation:
void ConcretePrinter::doPrint(char *str) { cout << str << endl; }
So far, that's fine. But now let's say I want to use a Decorator to add
some extra output:
class PrinterDecorator : public Printer
{
public:
virtual ~PrinterDecorator() {}
PrinterDecorator(Printer *decorated_printer) :
decorated_printer_(decorated_printer) {}
//...
private:
virtual void doPrint(char *str)
{
cout << "some decoration..." << endl;
decorated_printer_->print(str); // <-- line that bugs me
cout << "some decoration..." << endl;
}
Printer *decorated_printer_;
};
The print() call in this piece of code is what bugs me. I cannot call
decorated_printer_'s doPrint() because it is non-public in this context,
but calling print() means that all parameter checking performed in
Printer::print() is uselessly duplicated, and it would be duplicated
again for all further decorators or proxies I might add.

Why do you say "uselessly duplicated"? Isn't PrinterDecorator a
client of Printer, as well as being an implementation? Is the
author of PrinterDecorator somehow protected from making the
same stupid errors as other clients might make, and so not need
the same protection.
After all, by
the time PrinterDecorator::doPrint() is called, all necessary checking
already took place in Printer::print(). It's like the derived class
telling the base class, "I know you already checked the data, but please
check it again anyway."

No, it's like a client telling the service that it has already
checked the data, but please check it again anyway.
Granted, in this stupid example, it's just a pointer check, but think of
more expensive operations such as "do files exist", "can server be
accessed", or "lock for other threads". A program that duplicates such
operations "by design" doesn't strike me as very well designed.

Presumably, however, this holds for all checks. If the contract
says you should not call the function with a null pointer,
presumably, all clients will take steps to ensure that the
pointer they pass is not null. You check it anyway, because you
don't trust the client. Why is the client PrinterDecorator any
different? The fact that it also derives from Printer doesn't
mean that its authors are infallible.

Of course, if the profiler shows that this is a problem, you
could define an AbstractPrinterDecorator from which all
PrinterDecorator derive, declare it a friend of Printer, and
provide a function in it to forward to the unchecked version in
the target Printer. But I while an AbstractPrinterDecorator
isn't necessarily a bad idea in itself (since all
PrinterDecorator's have the common behavior of holding a pointer
to the decorated Printer object), I wouldn't start skipping any
of the intermediate checks until the profiler said I had to.
How do I cope with this situation? Is it just an unfortunate fact of
life that NVI and Decorator/Proxy-like patterns don't mix? Or am I
missing something?
In fact, I thought of a possible solution. If I gave Printer an
additional protected "print" method that took a Printer object and that
did *nothing* but delegate to doPrint(), I could call that protected
method from the decorator:

But this supposes that *all* Printer have the behavior of a
Decorator.
class Printer
{
//...
public:
void print(char *str); // clients of Printer keep calling this one
protected:
void print(Printer *printer, char *str) // <-- new method, to be

// called by derived classes
// if they need to
{
printer->doPrint(str);
}
private:
virtual void doPrint(char *str) = 0;
};
void PrinterDecorator::doPrint(char *str)
{
cout << "some decoration..." << endl;
print(decorated_printer_, str); // <-- now calling the new
// protected method
cout << "some decoration..." << endl;

}
Is this a good idea? It looks a bit confusing even to me although I came
up with it myself :)

Putting it in Printer is probably not a good idea, but I can see
it (and have done something similar in the past) in an abstract
base class of the decorators, which is a friend of Printer.
 
G

Greg Herlihy

My first reaction was to mention 'friendship' as well; however, a base
class shouldn't have to know its going to be 'decorated'. The whole
idea of the 'decorator' pattern is to embellish an existing
implementation without its knowledge.

And the Printer class still "knows" nothing about a PrinterDecorator -
except that the class (which does not even have to exist) is its
friend.

And since all of these classes are being developed at the same time
with the goal that they all work well together, I can't think of any
reason why PrinterDecorator would not be made a friend of Printer -
especially when all of the design decisions made up to this point
require that PrinterDecorator have access to Printer's private or
protected methods.

Greg
 
G

gpuchtel

And since all of these classes are being developed at the same time
with the goal that they all work well together, I can't think of any
reason why PrinterDecorator would not be made a friend of Printer -
especially when all of the design decisions made up to this point
require that PrinterDecorator have access to Printer's private or
protected methods.

Okay, but the question and my response were more fundamental than
that. That is, more fundamental regarding the behavior between his
decorator class and his decorated (base) class. What happens if
classes are not developed together? What if I want my classes to be
open to extention but closed to modification, the Open-Closed
principle (OCP). What if the base class is decorated by yet another
class? You would have to modify the base class in this case thereby
violating the OCP.

If you are saying that these classes will be developed at the same
time, one time, then why bother with the decorator pattern at all? In
that case simply create a new derived class. Then intent of the
'decorator' pattern it to add additional behavior to an existing
object. That is not what you are describing when you say there all
being developed at the same time. As I said, my first reaction was
'friendship' too; however, the original question was in the context of
the 'decorator' pattern, which has a different set of assumptions.
 
C

Christian Hackl

Greg Herlihy ha scritto:
The solution is to declare PrinterDecorator a friend of the Printer
class. As a friend of the Printer class, PrinterDecorator's doPrint()
method will be able to call doPrint() on its wrapped Printer object:

Thanks for your suggestion, but as "gpuchtel" already stated in the
other reply, I don't think this would be an optimal solution, given that
I seek to reduce dependencies.

I might not even know yet if I want to use a decorator or something
similar later in the project. Using friendship to achieve my goal would
require me to edit the definition of the base class every time I
wanted to add a decorator-like subclass, and this is what I want to avoid.
Furthermore, doPrint() should really be declared protected instead of
private - since a derived class may wish to call its base class'
version of doPrint() in the course of executing its own doPrint().

Again I have to disagree. In my opinion, the semantics of those private
abstract methods is that only the base class knows how and when to call
them safely. By making them protected I'd give up some encapsulation in
allowing subclasses to call them in any way they like, wouldn't I?
 
C

Christian Hackl

James Kanze ha scritto:
Why do you say "uselessly duplicated"? Isn't PrinterDecorator a
client of Printer, as well as being an implementation? Is the
author of PrinterDecorator somehow protected from making the
same stupid errors as other clients might make, and so not need
the same protection.

Hmmm...

Good point, I guess. I must say I didn't think of it that way. So you
mean subclasses could do something like this:

void PrinterDecorator::doPrint(char *str)
{
decorated_printer_->print(0); // very contrived; but the method may
// do some more complicated stuff that
// could accidentally result in calling
// print() with a null pointer
}

But this supposes that *all* Printer have the behavior of a
Decorator.

You mean because that protected method would be put in the base class
even though it's not part of the abstraction shared by all subclasses,
hence violating OOP principles? I think I understand. You are right, on
second thought that isn't exactly very nice.
Putting it in Printer is probably not a good idea, but I can see
it (and have done something similar in the past) in an abstract
base class of the decorators, which is a friend of Printer.

I see. Well, thanks for all those insights ... I guess my error was
indeed that I considered "PrinterDecorator" a special client of
"Printer" regarding the function call, when it actually isn't. From that
new point of view, the duplicated checks don't seem so useless anymore.
 
G

Greg Herlihy

Okay, but the question and my response were more fundamental than
that. That is, more fundamental regarding the behavior between his
decorator class and his decorated (base) class. What happens if
classes are not developed together? What if I want my classes to be
open to extention but closed to modification, the Open-Closed
principle (OCP). What if the base class is decorated by yet another
class? You would have to modify the base class in this case thereby
violating the OCP.

If you are saying that these classes will be developed at the same
time, one time, then why bother with the decorator pattern at all? In
that case simply create a new derived class. Then intent of the
'decorator' pattern it to add additional behavior to an existing
object. That is not what you are describing when you say there all
being developed at the same time. As I said, my first reaction was
'friendship' too; however, the original question was in the context of
the 'decorator' pattern, which has a different set of assumptions.

Well, in the general case the PrinterDecorator should call the public
print() method of the wrapped printer object. After all, most "decoration"
involves modifying the input arguments before passing them on to the
wrapped object (or otherwise manipulating the wrapped object's state before
forwarding the call).

So in all likelihood, the call to the wrapped printer object's print()
method could well proceed quite differently than the call to the
PrinterDecorator's print() method. In other words, there is little reason to
expect that the second call to print() is a pointless repetition of the
first.

Greg
 
J

James Kanze

James Kanze ha scritto:

Good point, I guess. I must say I didn't think of it that way. So you
mean subclasses could do something like this:
void PrinterDecorator::doPrint(char *str)
{
decorated_printer_->print(0); // very contrived; but the method may
// do some more complicated stuff that
// could accidentally result in calling
// print() with a null pointer
}

Obviously, it could.

In the end, I think, the question is whether the decorators are
part of the same subsystem as Printer, developed with it, and
seen as an ensemble by other users, or whether they are
independent code, which is developed by other users. In the
first case, I'd probably go with an AbstractPrinterDecorator
base class, which was a friend of Printer; in the second, I'd
definitly require all accesses to Printer to go through the
standard public interface. I'm not about to let a user that I
don't know off the hook just because he chooses to name his
class Decorator:).
You mean because that protected method would be put in the base class
even though it's not part of the abstraction shared by all subclasses,
hence violating OOP principles?
Exactly.

I think I understand. You are right, on
second thought that isn't exactly very nice.
I see. Well, thanks for all those insights ... I guess my error was
indeed that I considered "PrinterDecorator" a special client of
"Printer" regarding the function call, when it actually isn't.

Well, it might be. There is no one right answer. If it is a
special client, privileged in some way, then friend is an
appropriate solution.

But it's a question I would definitly ask myself. As I said
above, just because some random user decides to name his class
Decorator (or use my class in a design pattern of his own)
shouldn't give him any special rights or trust.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top