On Nov 18, 3:02 pm, Steven D'Aprano
<ste...@remove.this.cybersource.com.au> wrote:
> Lexical duplication is one of the weakest code smells around, because it
> is so prone to false negatives. You often can't avoid referring to the
> same lexical element multiple times:
>
> def sinc(x):
> if x != 0:
> return sin(x)/x
> return 1
>
The existence of one code sample where lexical duplication is a false
negative should not get you into the habit of disregarding it all
together.
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. In my
example, the reference to _dict is 36 lines of code away from its
implementation, which forces indirection on the reader. So maybe the
methods in between def _dict and _dict are too long. I can pretty
quickly rule that out, as the methods average about four lines each.
So maybe the dispatch method is calling out the need for a class, as
Simon suggested in another post. I also wonder if an inline decorator
is in order.
One thing I promise not to do is revert the code to if/elif/elif. :)
--
http://mail.python.org/mailman/listinfo/python-list
