Emmanuel,

Comments in line ...

On 8/18/07, Emmanuel Lecharny <elecharny@gmail.com> 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 < akarasulu@apache.org> wrote:
> Hi all,
>
> While processing the JIRA I noticed an issue that I had fixed in ADS 1.0 but
> 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