Pattern usage and writing self documenting code

[Alex Karasulu <ak...@apache.org>]

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

Re: Pattern usage and writing self documenting code

[Alex Karasulu <ak...@apache.org>]

Emmanuel,

Comments in line ...

On 8/18/07, Emmanuel Lecharny <el...@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 <ak...@apache.org> 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
>

Re: Pattern usage and writing self documenting code

[Emmanuel Lecharny <el...@gmail.com>]

Hi ALex,

I can understand your position, and furthermore, you know how I hate
patterns because they are conveying some kind of semantic which is
often over abused. I know a lot of people who are coding using
patterns everywhere thinking that patterns is a good substitute for
design. Now, people often confuse patterns and GoF, thinking that
there are no other patterns thet the ones described in this Sacred
Book.

Pattern are good to use if they are commonly used by a group of
people, and understood. Otherwise, they are always cryptic, and
whatever self-documentation you add, you will end by browsing the
internet crazy finding a zillion of different explanations about what
is the offending pattern all about. I see patterns as a good
justification for people who don't want to think, when they are
abused...

Patterns are by no mean the solution to every programming problem,
they should be used in clearly identified cases.

In this very special case, what was done is to cretae a separation
between the action and the transport. 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 won't call it J2ee-ish, but MVC-ish, which is not by far a bad
pattern ...

At the end, yes, this is far more complex than the previous code. But
everything has a price, and you can just think that enter in a code
which is complex will be at no price. 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 understand that we can have different visions of how should be
written a piece of code, 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.

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 !

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 !

Emmanuel

On 8/18/07, Alex Karasulu <ak...@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