This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author jimjjewett
Recipients
Date 2007-04-24.13:53:30
SpamBayes Score
Marked as misclassified
Message-id
In-reply-to
Content
I haven't looked deeply yet, but I know some feedback quickly is often better than a long wait.  I do have a few style suggestions.

(1)  Tests for None should generally use "is" rather than "==".

(2)  It helps to understand the code if it is more parallel.  Seeing the meth is/is not None cases reversed on the two orders confused me about how similar they were.

(3)  Inheriting from object (or setting __metaclass__ = type) is probably a good habit, since old-style classes will disappear in Py3K.  Normally, it doesn't matter, but debugging the difference is a pain.

To make the first two concrete, here is my rewording of ASTVisitor.dispatch:

From:

    def dispatch(self, node, *args):
        klass = node.__class__
        meth = self._get_method_for_className(klass)
        if self._order == self.PREORDER:
            if meth != None and meth(node, *args) != self.NO_CONTINUE:
                self._iterate_over_defaults(node, *args)
            elif meth == None:
                self._iterate_over_others(node, *args)
        elif self._order == self.POSTORDER:
            if meth == None:
                self._iterate_over_others(node, *args)
            if meth != None:
                self._iterate_over_defaults(node, *args)
                meth(node, *args)

To:

    def dispatch(self, node, *args):
        klass = node.__class__
        meth = self._get_method_for_className(klass)
        if meth is None:
            self._iterate_over_others(node, *args)
        else:
            if self._order == self.POSTORDER:
                self._iterate_over_defaults(node, *args)
                meth(node, *args)
            elif meth(node, *args) != self.NO_CONTINUE:
                self._iterate_over_defaults(node, *args)
            else:
                pass # pre-order, and the method said to stop

History
Date User Action Args
2007-08-23 15:58:13adminlinkissue1706323 messages
2007-08-23 15:58:13admincreate