Emmanuel,
Comments in line ...
On 8/18/07, Emmanuel Lecharny wrote:
SNIP ...
Patterns are by no mean the solution to every programming problem,
> they should be used in clearly identified cases.
+1
I could not agree with you more!
In this very special case, what was done is to cretae a separation
> between the action and the transport.
See that's where it becomes confusing to me. Call me a simpleton but this
is about
writing some commands for a CLI utility program. Why introduce transport
abstraction
into a simple tool intended for the command line?
Action will remain the same,
> whatever the used transport. Of course, this will clutter the initial
> code, as we have added this transport layer, when the initial code was
> just about writing a locally active tool (you were just able to use
> the tools on the same machine than the server). Now, the idea is to
> able someone using a tool like Apache Directory Studio to do the same
> thing, possibly communicating with the server using a HTTP transport.
I think there is far too much overloading going on in this simple project
which
makes a small thing very hard to manage. Did anyone even use this code in
Studio for remote command execution? If we don't use it and still pay the
cost
of more complex CLI tools then that's a waste of time and energy.
Secondly, this particular command, the index command, as well as others like
dump, are intended for offline use. So some commands are not going to
support
this Action/Transport pattern and the result will be special cases all over
the code.
Why apply this pattern to the entire CLI based apacheds-tools project when
only
some of the commands might benefit from it. This is what I mean about trying
to
force a pattern where it does not fit. Trying to reuse code is nobel and I
commend
PAM for trying to do this. Yet in this case it is not resulting in any net
gain. If you
wrote two distinct peices of code for the different use cases then it still
will probably
be less code to maintain but most importantly it will be simpler because you
won't
have special handling for the one offs that breach the pattern.
Patterns are good but often we incorrectly apply them where they don't
belong. In this
case we are mis using the pattern in an attempt to reuse code. If you take
a step back
the effort is not worth it.
At the end, yes, this is far more complex than the previous code.
Oh yes - this is why I could not modify it quickly. It's been turned into a
something far
too complex for me.
But
> everything has a price, and you can just think that enter in a code
> which is complex will be at no price.
Sometimes you don't need to complicate something by trying to force reuse in
different situations. Just write another implementation. People go too far
to try to
reuse code at the cost of complexity and this is a perfect example of it.
Let me ask you a question: would you rather have two simple yet similar
peices of
code to manage or one peice of complex code that handles all scenarios? I
prefer
the simpler code which eventually produces less bugs even though some
fragments
of code overlap. And after you write the two peices of code then you might
step back
and try to see where we can consolidate and enable reuse. It's a risky to
presume
from the onset that something should be reuse before knowing what things
will look
like. This is why extreme programming concepts tell coders to stop thinking
so much
and just implement then refactor.
The choice here is between
> simplistic and limitated code, or more complex but potentially more
> powerful code. I don't think we over abused patterns in this very
> case, but of course this can be discussed.
I can't agree with you on this. Just think about the net result. You lost
me
as a maintainer of this code. Why? Because it's now too complex for me to
manage in what I would deem reasonable time. Now I have to think of twice
as many things while going in there when all I want to do is add another
simple
command for CLI usage.
I understand that we can have different visions of how should be
> written a piece of code,
Generally this is true however in this specific case, with our positions in
this discussion,
one cannot chalk this up to vision or differences in opinion. We can measure
these things
and see one way will result in less energy and time wasted while keeping the
door open
for more simpletons like myself to contribute.
but trust me, this one has been seriously
> discussed before being implemented, and as I didn't implemented it nor
> choose the implementation, I'm more confident to defend it than if I
> was the author. There might be room for improvement, for sure (names
> used may not be perfect), but I really think that the used pattern
> (proxy) is the correct one for this very case.
It's not the pattern only in this case. The pattern might work for the
remote
execution aspect. If you only made apacheds-tools a remote command
triggering
facility then this might be a good fit. However you're mixing this pattern
with
both CLI and remote execution which is resulting in too much complexity.
If you had the services in place to execute the commands over the wire then
I would just have the apacheds-tools utility make calls over the wire even
to
ApacheDS instances running locally. However this still does not work since
some functionality will need to be executed even when the server is down
like
the dump command. It just does not make sense to force this pattern across
all
functionality.
IMO too much is being pushed into this code to make it do too many different
things. I would use the UNIX philosophy of writing a solid simple tool that
does
one thing but very well.
let's wait for pam to be back from hollidays to further this
> discussion, he is the one who is the best to explain what has been
> done !
NP - this is no big deal we just need to talk about it to understand the
best way.
Thanks for your comment anyways, Alex, I'm sure we will find a better
> way to code this part when all the elements will be put on the table !
Oh yeah definitely. There are so many dimensions to writing code that no
two
developers can agree on everything all of the time. This is not an argument
but
a discussion where we disagree about what to do. With more discorse we can
flush out the right answer. This is why I wrote this email :).
> On 8/18/07, Alex Karasulu wrote:
> > Hi all,
> >
> > While processing the JIRA I noticed an issue that I had fixed in ADS 1.0but
> > did not merge
> > into the trunk for ADS 1.5. I am referring to the following JIRA issue
> > here:
> >
> > https://issues.apache.org/jira/browse/DIRSERVER-922
> >
> > I remember distinctly trying to merge these changes to the trunk after
> > implementing the fix in the 1.0
> > branch however it was impossible to merge because the apacheds-tools
> project
> > was completely
> > refactored with a new design. While looking at that code I could not
> easily
> > understand it.
> > After the fact though someone pointed me to some documentation on the
> > redesign that took place.
> > It would have been nice to have this link at that point when I had to do
> the
> > merge but this is not
> > always possible. Hence my point as to why writing self documenting code
> is
> > critical.
> >
> > The redesign added a bunch of patterns with unclear names which was
> > confusing to me.
> > Besides being somewhat J2EE-ish (which I don't like) the patterns were
> > somewhat off. I could
> > not get familiar enough with it in a reasonable amount of time to fix
> the
> > conflicts I encountered
> > on the merge. Again I encountered this issue in JIRA and the same
> problem is
> > still there so I
> > re-assigned the issue to who SVN reported as the redesigner.
> >
> > My point to all this is that we must chose names properly for elements
> in
> > patterns and
> > make sure we use patterns properly instead of tweaking them to the point
> of
> > no recognition.
> > Is'nt conveying design through instant recognition the point to using
> > patterns in the first
> > place. There is no point to using patterns unless you abide by them.
> > Furthermore it's
> > not just enough to write documentation on your design or redesign if the
> > names are funky
> > and the patterns are warped to force a fit. This is actually a worse
> > practice. If you don't
> > try to force the pattern then pattern names would not be mixed with
> clear
> > names
> > for the classes used. Mixing them and changing the pattern to suite is
> > creating more
> > confusion while removing the effectiveness of using the pattern in the
> first
> > place.
> >
> > I'm seeing this practice repeatedly and it's starting to consume more of
> my
> > time and
> > is causing me to take actions that I don't like: for example I just
> pushed
> > this JIRA issue
> > above to PAM instead of stepping up to the plate and fixing it
> myself. This
> > is all
> > because groking this redesign in the time alotted was impossible for me.
> > This practice
> > is making the code unfamiliar to those who were capable before of
> navigating
> > it.
> >
> > This criticism is intended to be constructive so please don't anyone
> take it
> > personally.
> > This is a problem that I have been noticing and dealing with in various
> > places. This
> > particular example above is just one of those instances. I myself have
> > made this mistake
> > all too often and others have grok'd through my cryptic code so I have
> to
> > take my own
> > advice. I just want to speak up since our projects are getting more
> complex
> > every day.
> >
> > Alex
> >
> >
>
>
> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>