Code review comment for lp:~sinzui/launchpad/merge-karma-1

On Tue, Nov 2, 2010 at 8:11 AM, Robert Collins
<email address hidden> wrote:
> That would be a bug. The correct spelling is either
> if result:
> or
> if result is not None and len(result) > 0:
>
> if result:
> is faster and logically equivalent - writing it long hand gains nothing.

Blah, I didn't mean to be quite so black-and-white there.

I know we have a coding style guideline that said to avoid 'if foo:',
but there were two issues in the review:
- the sense of the test was inverted (from if foo to if is None)
- the change from __nonzero__ to a less complete condition which can
break if the list is empty.

The former one obviously matters most. The __nonzero__ angle I haven't
had the time to debate in our style guide yet, though I do feel
strongly about it - we've had numerous bugs because we *didn't use*
__nonzero__ for tests. We've also had bugs because we did :) - I think
we will benefit by removing the guideline altogether and just using
the most appropriate spelling at any one place.

-Rob