On Nov 18, 5:42 pm, Steven D'Aprano
<ste...@remove.this.cybersource.com.au> wrote:
> On Wed, 18 Nov 2009 17:14:27 -0800, Steve Howell wrote:
> > In my rewritten code, here is the smell:
>
> >     dispatches = {
> >             'dict': _dict,
> >             'list': _list,
> >             'attr': _attr,
> >             'key': _key,
> >             'as': _as,
> >             'call': _call,
> >             'recurse': _recurse,
> >             }
> >     if kind in dispatches:
> >         return dispatches[kind](ast)
> >     else:
> >         raise Exception('unknown kind!')
>
> > There is nothing wrong with the code taken out of context, but one of
> > the first things that lexical duplication should alert you to is the
> > fact that you are creating code that is brittle to extension.
>
> Really? It is very simple to extend it by adding another key:item to the
> dict.
>

Easy for me to extend, of course, since I wrote the code.  It's the
maintainers I am worried about.  I can make their lives easier, of
course, by changing the text of the exception to say something like
this:

   "Silly maintenance programmer, you now have to update the dispatch
table!"

I have no problem with that.


> > In my
> > example, the reference to _dict is 36 lines of code away from its
> > implementation, which forces indirection on the reader.
>
> It gets worse -- the reference to the in operator is in a completely
> different module to the implementation of the in operator, which may be
> in a completely different language! (C in this case.)
>
> I'm being sarcastic, of course, but I do have a serious point. I'm not
> impressed by complaints that the definition of the function is oh-so-very-
> far-away from where you use it. Well duh, that's one of the reasons we
> have functions, so they can be used anywhere, not just where they're
> defined.
>

The slippery slope here is that every block of code that you ever
write should be extracted out into a method.  Now I am all in favor of
tiny methods, but the whole point of if/elif/elif/elif/elif/end code
is that you are actually calling out to the maintenance programmer
that this code is only applicable in certain conditions.  That's why
"if" statements are called "conditionals"!  You are actually telling
the maintenance programmer that is only sensible to use these
statements under these conditions.  It is actually extremely explicit.

Now I'm being sarcastic too, but I also have a serious point.  Nine
times out of ten if/elif/elif/end calls out the smell of spaghetti
code.  But it can also occasionally be a false negative.  It can be a
case of avoiding premature abstraction and indirection.


-- 
http://mail.python.org/mailman/listinfo/python-list

Reply via email to