| View previous topic :: View next topic |
| Author |
Message |
ManicQin Guest
|
Posted: Tue Aug 26, 2008 8:18 am Post subject: 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! |
|
| |
|
Back to top |
ManicQin Guest
|
Posted: Tue Aug 26, 2008 12:32 pm Post subject: Re: is visitor the best solution? |
|
|
On Aug 26, 1:48 pm, Patrick May <p...@spe.com> wrote:
| Quote: | 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.
| Quote: | 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? |
|
| |
|
Back to top |
ManicQin Guest
|
Posted: Tue Aug 26, 2008 1:11 pm Post subject: Re: is visitor the best solution? |
|
|
On Aug 26, 2:32 pm, ManicQin <Manic...@gmail.com> wrote:
| Quote: | 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... |
|
| |
|
Back to top |
ManicQin Guest
|
Posted: Tue Aug 26, 2008 2:09 pm Post subject: Re: is visitor the best solution? |
|
|
On Aug 26, 3:16 pm, Patrick May <p...@spe.com> wrote:
| Quote: |
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? |
|
| |
|
Back to top |
ManicQin Guest
|
Posted: Tue Aug 26, 2008 2:48 pm Post subject: Re: is visitor the best solution? |
|
|
On Aug 26, 4:09 pm, ManicQin <Manic...@gmail.com> wrote:
| Quote: | 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... |
|
| |
|
Back to top |
Patrick May Guest
|
Posted: Tue Aug 26, 2008 4:48 pm Post subject: Re: is visitor the best solution? |
|
|
ManicQin <ManicQin@gmail.com> writes:
[ . . . ]
| Quote: | 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) |
|
| |
|
Back to top |
Patrick May Guest
|
Posted: Tue Aug 26, 2008 6:16 pm Post subject: Re: is visitor the best solution? |
|
|
ManicQin <ManicQin@gmail.com> writes:
| Quote: | 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.
| Quote: | 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.
| Quote: | 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.
| Quote: | 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) |
|
| |
|
Back to top |
H. S. Lahman Guest
|
Posted: Tue Aug 26, 2008 10:31 pm Post subject: Re: is visitor the best solution? |
|
|
Responding to ManicQin...
| Quote: | 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 |
|
| |
|
Back to top |
Patrick May Guest
|
Posted: Wed Aug 27, 2008 4:18 am Post subject: Re: is visitor the best solution? |
|
|
ManicQin <ManicQin@gmail.com> writes:
| Quote: | 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) |
|
| |
|
Back to top |
ManicQin Guest
|
Posted: Wed Aug 27, 2008 8:42 am Post subject: Re: is visitor the best solution? |
|
|
Thank you very much Patrick and Lahman sorry for Top postings
On Aug 27, 1:18 am, Patrick May <p...@spe.com> wrote:
| Quote: | 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 .
| Quote: | 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)
| Quote: |
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!!!! |
|
| |
|
Back to top |
ManicQin Guest
|
Posted: Wed Aug 27, 2008 4:06 pm Post subject: Re: is visitor the best solution? |
|
|
Patrick May wrote:
| Quote: | ManicQin <ManicQin@gmail.com> writes:
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 .
I agree that it's a hack. Archiving is an additional capability
to be added to the basic logging functionality. The logger itself
should just do logging.
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?
The client creates the logger they want with something like:
logger = new PhaseOfMoonArchiver(new ConcreteLogger());
then uses it like any other logger. The PhaseOfMoonArchiver wraps the
logger functionality and performs the archiving whenever necessary.
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 you'll find that building composites like this will be at
least as intuitive as passing policies into a logger class. Each
class will have a single responsibility, easy to understand and test
in isolation.
By the way, this is exactly the type of "small" problem that can
result in a tangle, untestable implementation unless you build it test
first. Writing the tests will also allow you to try different
interfaces and let the code lead you to the way it wants to be
written.
|
At first I continued with my despicable way... until I hit a wall and
realised the benefits of your method.... Thanks |
|
| |
|
Back to top |
Patrick May Guest
|
Posted: Wed Aug 27, 2008 4:37 pm Post subject: Re: is visitor the best solution? |
|
|
ManicQin <ManicQin@gmail.com> writes:
| Quote: | 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 .
|
I agree that it's a hack. Archiving is an additional capability
to be added to the basic logging functionality. The logger itself
should just do logging.
| Quote: | 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?
|
The client creates the logger they want with something like:
logger = new PhaseOfMoonArchiver(new ConcreteLogger());
then uses it like any other logger. The PhaseOfMoonArchiver wraps the
logger functionality and performs the archiving whenever necessary.
| Quote: | 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 you'll find that building composites like this will be at
least as intuitive as passing policies into a logger class. Each
class will have a single responsibility, easy to understand and test
in isolation.
By the way, this is exactly the type of "small" problem that can
result in a tangle, untestable implementation unless you build it test
first. Writing the tests will also allow you to try different
interfaces and let the code lead you to the way it wants to be
written.
| Quote: | 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!!!!!!!!
|
That's a reason to come up with a template solution right there.
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) |
|
| |
|
Back to top |
Patrick May Guest
|
Posted: Wed Aug 27, 2008 9:13 pm Post subject: Re: is visitor the best solution? |
|
|
ManicQin <ManicQin@gmail.com> writes:
| Quote: | At first I continued with my despicable way... until I hit a wall
and realised the benefits of your method.... Thanks
|
*laugh* Dude, it's all just code. Nothing despicable -- just
engineering tradeoffs.
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) |
|
| |
|
Back to top |
|