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