is visitor the best solution?

Hello everybody, I'm trying to "polish" an old class in one of our
projects...
the class is a logger and he is being derived by many projects and
specialized for their needs
one of the loggers features is to auto detect when the file should be
archived and a new file should be opened... For some strange reason
this function is pure virtual in the base class and every deriver
(15!) just copy pastes the implementation from the first implementor,
except one deriver that really has special needs ... (and offcourse
after 5 years of bugs the implemantations vary because once a bug was
found in one of the projects implemantation no one had the sense of
telling the others that they should fix it... classic isn't it?)

As I was saying I decided to "polish" the class, I wanted to
encapsulate the auto archive policies and to load them according to
configuration\needs.

the problem is: let's take two policies
1) End of Day
2) bytes written

Each one of them decideds what to do according to different types and
values
(let's say date m_date and ulonglong m_bytes) so a unified member
function is a bit difficult to produce, which gives me two other
options
1) to move the values into the policy class (I dont like this
solution)
2) to give the policies a "free hand" to the members of the logger by
passing them an instance of the logger ... Isnt it considered a
visitor?

Do you have any better "easy to implement in an ill formed class"
solution to the problem?
Was I right to diagnose the second solution as a visitor?
Thanks!

ManicQin <ManicQin@gmail.com> writes:
[ . . . ]
> As I was saying I decided to "polish" the class, I wanted to
> encapsulate the auto archive policies and to load them according to
> configuration\needs.
>
> the problem is: let's take two policies
> 1) End of Day
> 2) bytes written
>
> Each one of them decideds what to do according to different types
> and values (let's say date m_date and ulonglong m_bytes) so a
> unified member function is a bit difficult to produce, which gives
> me two other options
>
> 1) to move the values into the policy class (I dont like this
> solution)
> 2) to give the policies a "free hand" to the members of the logger by
> passing them an instance of the logger ... Isnt it considered a
> visitor?

If I understand your requirements correctly, what you really want
to do is specify the archive function at runtime. Does the language
you're using support first class functions or some hacky variant
thereof?

Your second solution isn't technically a Visitor. The Visitor
pattern involves adding an accept(Visitor) method to the logger. It
is used to add new functionality to a related group of classes in
languages that don't allow that by default. The Visitor is passed to
each element in a set of instances of those classes.

Regards,

Patrick

------------------------------------------------------------------------
S P Engineering, Inc. | Large scale, mission-critical, distributed OO
| systems design and implementation.
pjm@spe.com | (C++, Java, Common Lisp, Jini, middleware, SOA)

On Aug 26, 1:48*pm, Patrick May <p...@spe.com> wrote:

> * * *If I understand your requirements correctly, what you really want
> to do is specify the archive function at runtime. *Does the language
> you're using support first class functions or some hacky variant
> thereof?

To be more specific the classes would decide if the log should be
archived
the operation of archiving is "easy" and for now should not be
derived.
(I know that maybe it's the same in your POV but still I wanted to
make it clear)

AFAIK c++ support first class functions.

> * * *Your second solution isn't technically a Visitor. *The Visitor
> pattern involves adding an accept(Visitor) method to the logger. *It
> is used to add new functionality to a related group of classes in
> languages that don't allow that by default. *The Visitor is passed to
> each element in a set of instances of those classes.

Maybe I didnt underatnd you but I always thought that visitor
should be used when you want to add functionality to a class
by extracting it from the class into a new one but still using
members
variables of the main class (which sounds like friend class...)

Can you think of any better idea?

On Aug 26, 2:32*pm, ManicQin <Manic...@gmail.com> wrote:

> Maybe I didnt underatnd you but I always thought that visitor
> should be used when you want to add functionality to a class
> by extracting it from the class into a new one but still using
> members
> variables of the main class (which sounds like friend class...)
>
> Can you think of any better idea?

Ok... after further research of the visitor pattern I got that it is
not what I thought it was...

ManicQin <ManicQin@gmail.com> writes:
> On Aug 26, 1:48*pm, Patrick May <p...@spe.com> wrote:
>> If I understand your requirements correctly, what you really
>> want to do is specify the archive function at runtime. Does the
>> language you're using support first class functions or some hacky
>> variant thereof?
>
> To be more specific the classes would decide if the log should be
> archived the operation of archiving is "easy" and for now should not
> be derived. (I know that maybe it's the same in your POV but still
> I wanted to make it clear)

I think what you want is the Strategy pattern, then. Your
clients can pass the specific strategy class to the constructor of the
logger.

> AFAIK c++ support first class functions.

Not really, since it isn't possible to construct new functions at
runtime, but C++ does at least support pointers to functions. That's
an alternative to a strategy class.

>> Your second solution isn't technically a Visitor. The Visitor
>> pattern involves adding an accept(Visitor) method to the logger.
>> It is used to add new functionality to a related group of classes
>> in languages that don't allow that by default. The Visitor is
>> passed to each element in a set of instances of those classes.
>
> Maybe I didnt underatnd you but I always thought that visitor should
> be used when you want to add functionality to a class by extracting
> it from the class into a new one but still using members variables
> of the main class (which sounds like friend class...)

Visitors don't necessarily have to have access to the internals
of the class they visit. They can use the exposed public API.

> Can you think of any better idea?

From what you've described, I think you can pass either a
strategy class or a pointer to a function to the constructor of your
logger to achieve what you want. Will either of those work?

Regards,

Patrick

------------------------------------------------------------------------
S P Engineering, Inc. | Large scale, mission-critical, distributed OO
| systems design and implementation.
pjm@spe.com | (C++, Java, Common Lisp, Jini, middleware, SOA)

On Aug 26, 3:16*pm, Patrick May <p...@spe.com> wrote:

>
> * * *From what you've described, I think you can pass either a
> strategy class or a pointer to a function to the constructor of your
> logger to achieve what you want. *Will either of those work?
>

Doesn't both methods force me to use the same signature for the
functions?
My policies are based on different types.

I could pass a struct such as policyArg to the function...
let's "attack" End Of Day policy so we would have a sEODpolicy :
policyArg
with the memeber variable of day m_Date
(and the shallow copy \ clone function)
the function would be Decide_If_to_Archive(policyArg* blabla)
and she would yank the information through the
struct and decide if to archive or not...

sounds good?

On Aug 26, 4:09*pm, ManicQin <Manic...@gmail.com> wrote:
> On Aug 26, 3:16*pm, Patrick May <p...@spe.com> wrote:
>
>
>
> > * * *From what you've described, I think you can pass either a
> > strategy class or a pointer to a function to the constructor of your
> > logger to achieve what you want. *Will either of those work?
>
> Doesn't both methods force me to use the same signature for the
> functions?
> My policies are based on different types.
>
> I could pass a struct such as policyArg to the function...
> let's "attack" End Of Day policy so we would *have a sEODpolicy :
> policyArg
> with the memeber variable of day m_Date
> (and the shallow copy \ clone function)
> the function would be Decide_If_to_Archive(policyArg* blabla)
> and she would yank the information through the
> struct and decide if to archive or not...
>
> sounds good?

But this solution is impossible to automate...

Responding to ManicQin...

> Hello everybody, I'm trying to "polish" an old class in one of our
> projects...
> the class is a logger and he is being derived by many projects and
> specialized for their needs
> one of the loggers features is to auto detect when the file should be
> archived and a new file should be opened... For some strange reason
> this function is pure virtual in the base class and every deriver
> (15!) just copy pastes the implementation from the first implementor,
> except one deriver that really has special needs ... (and offcourse
> after 5 years of bugs the implemantations vary because once a bug was
> found in one of the projects implemantation no one had the sense of
> telling the others that they should fix it... classic isn't it?)

Yech. This is just Way Bad implementation of generalization and you
don't need Visitor to fix it.

Define the common implementation as a private method of the appropriate
superclass. Have the implementations of the public virtual method invoke
the private method in each of the subclasses that will the same
processing. Then provide a specific implementation of the virtual method
in the subclass that needs to do things differently.

[Super]
+ doIt()
- commonDoIt()
A
|
+----------+-------------+---....
| | |
[UniqueSub] [CommonSub1] [CommonSub2]


Class Super {
public:
virtual void doIt();
private:
void commonDoIt();
}

Super::comonSub1 () {
// implement common behavior
}

void CommonSub1::doIt() {
Super::commonDoIt();
}

void UniqueSub::doIt() {
// implement unique behavior
}

There's no implementation inheritance per se so you don't need to worry
about breaking existing clients if the tree is reorganized during
maintenance. For the individual subclasses that share the common
implementation, the implementation of the public virtual method is a
trivial indirection. Better yet, it is easy to determine what
implementation is used by simply looking at the subclass implementation.
So if requirements change and some subset of the original
common-behavior subclasses need a different implementation in the
future, it is easy to do by replacing the indirection.


--
There is nothing wrong with me that could
not be cured by a capful of Drano.

H. S. Lahman
hsl@pathfindermda.com
Pathfinder Solutions
http://www.pathfindermda.com
blog: http://pathfinderpeople.blogs.com/hslahman
"Model-Based Translation: The Next Step in Agile Development". Email
info@pathfindermda.com for your copy.
Pathfinder is hiring:
http://www.pathfindermda.com/about_us/careers_pos3.php.
(888)OOA-PATH

ManicQin <ManicQin@gmail.com> writes:
> On Aug 26, 3:16 pm, Patrick May <p...@spe.com> wrote:
>> From what you've described, I think you can pass either a
>> strategy class or a pointer to a function to the constructor of
>> your logger to achieve what you want. Will either of those work?
>
> Doesn't both methods force me to use the same signature for the
> functions?
> My policies are based on different types.

Yes, but I don't think that's a problem in practice. Let's try
this with (untested, uncompiled, possibly uncompilable) code:

class Logger
{
private:
ArchiveStrategy* archiveStrategy_;
long bytesWritten_;

public:
Logger(const ArchiveStrategy* archiveStrategy)
: archiveStrategy_(archiveStrategy), bytesWritten_(0) { }
virtual ~Logger() { }

virtual long bytesWritten() const { return bytesWritten; }

virtual void log(string message)
{
// logging behavior

if (archiveStrategy_->archive(this))
// archiving behvior
}
}

class ArchiveStrategy
{
public:
ArchiveStrategy() { }
virtual ~ArchiveStrategy() { }

virtual bool archive(const Logger* logger) = 0;
}

Then in the client you create the logger and archive strategy subtypes
you want:

logger = new ConcreteLogger(new PhaseOfMoonArchiveStrategy());

You could use function pointers instead of strategy classes.

Even with proper use of smart pointers and references, I don't
like this. Calling the strategy explicitly is a hack (in the sense of
building furniture with an axe). Wrapping the logger in the archiving
strategy is cleaner:

class Logger
{
private:
long bytesWritten_;

public:
Logger() : bytesWritten_(0) { }
virtual ~Logger() { }

virtual long bytesWritten() const { return bytesWritten; }

virtual void log(string message)
{
// logging behavior
}
}

class ArchivedLogger : public Logger
{
private:
Logger* logger_;

public:
ArchivedLogger(const Logger* logger) : logger_(logger) { }
virtual ~ArchiveStrategy() { }

virtual bool log(string message) = 0;
}

This (suitably cleaned up) let's the client add archiving
functionality on the fly:

logger = new PhaseOfMoonArchivedLogger(new ConcreteLogger());

Does this do what you want?

There's probably an elegant way to do this with templates, but
I've been rotting my brain with Java all day and need to go kick a
puppy.

Regards,

Patrick

------------------------------------------------------------------------
S P Engineering, Inc. | Large scale, mission-critical, distributed OO
| systems design and implementation.
pjm@spe.com | (C++, Java, Common Lisp, Jini, middleware, SOA)

Thank you very much Patrick and Lahman sorry for Top postings

On Aug 27, 1:18*am, Patrick May <p...@spe.com> wrote:
> ManicQin <Manic...@gmail.com> writes:
> > On Aug 26, 3:16 pm, Patrick May <p...@spe.com> wrote:
> >> * * *From what you've described, I think you can pass either a
> >> strategy class or a pointer to a function to the constructor of
> >> your logger to achieve what you want. *Will either of those work?
>
> > Doesn't both methods force me to use the same signature for the
> > functions?
> > My policies are based on different types.

I didnt see that you are talking about passing a pointer to the logger
class.
I thought that you mean that each policy will recieve it's own data as
in
ArchiveMoonPhase::Archive(date newDate);
ArchiveBytesWritten::archive(long bytesWritte);
Which Like I said breaks the signature...

My main problem is that passing a pointer to the log class to the
policies looks like a hack..
you know what I mean? But the more I think about it the more it looks
like the problem only resideds in my brain .

> class Logger
> {
> private:
> * ArchiveStrategy* archiveStrategy_;
> * long bytesWritten_;
>
> public:
> * Logger(const ArchiveStrategy* archiveStrategy)
> * * : archiveStrategy_(archiveStrategy), bytesWritten_(0) { }
> * virtual ~Logger() { }
>
> * virtual long bytesWritten() const { return bytesWritten; }
>
> * virtual void log(string message)
> * * {
> * * // logging behavior
>
> * * if (archiveStrategy_->archive(this))
> * * * // archiving behvior
> * * }
>
> }
>
> class ArchiveStrategy
> {
> public:
> * ArchiveStrategy() { }
> * virtual ~ArchiveStrategy() { }
>
> * virtual bool archive(const Logger* logger) = 0;
>
> }
>
> Then in the client you create the logger and archive strategy subtypes
> you want:
>
> * logger = new ConcreteLogger(new PhaseOfMoonArchiveStrategy());
>
> You could use function pointers instead of strategy classes.
>
> * * *Even with proper use of smart pointers and references, I don't
> like this. *Calling the strategy explicitly is a hack (in the sense of
> building furniture with an axe). *Wrapping the logger in the archiving
> strategy is cleaner:
>
> class Logger
> {
> private:
> * long bytesWritten_;
>
> public:
> * Logger() : bytesWritten_(0) { }
> * virtual ~Logger() { }
>
> * virtual long bytesWritten() const { return bytesWritten; }
>
> * virtual void log(string message)
> * * {
> * * // logging behavior
> * * }
>
> }
>
> class ArchivedLogger : public Logger
> {
> private:
> * Logger* logger_;
>
> public:
> * ArchivedLogger(const Logger* logger) : logger_(logger) { }
> * virtual ~ArchiveStrategy() { }
>
> * virtual bool log(string message) = 0;
>
> }

I missed something in your code,
*) the log pure vrtual function is at the deriver?!?
*) than who calls the archive function?
This method arises a problem when I want to use few policies on the
same logger...
converting it into a compositor that holds the logger looks wierd a
bit clumsy.
And I always prefer writing an "idiot proof" code this method has too
many pitfalls.

I think I'll give the logger a pointer to a policy compositor that
recieves a pointer to the logger itself... (the same thing I tried to
avoid from the start).
Sounds reasonable?
(Oh cr*p writing a compositor without STL... So so so pointless)

>
> This (suitably cleaned up) let's the client add archiving
> functionality on the fly:
>
> * logger = new PhaseOfMoonArchivedLogger(new ConcreteLogger());
>
> Does this do what you want?
>
> * * *There's probably an elegant way to do this with templates, but
> I've been rotting my brain with Java all day and need to go kick a
> puppy.

Whenever I give my boss a solution with templates he runs amok...
C with classes Is the sh** YHE!!!!!!!!

THANKS!!!!