Re: [StGit PATCH v2 3/6] stg mail: make __send_message do more

From: Karl Wiberg <kha@treskal.com>
Date: Dec 4, 2009, 07:00 UTC
Message-ID: <b8197bcb0912032300v62b8764bh7bff7e273a4792f5@mail.gmail.com>
In-Reply-To: <20091203193018.GF23258@ldl.fc.hp.com>

On Thu, Dec 3, 2009 at 8:30 PM, Alex Chiang <achiang@hp.com> wrote:

Show 22 quoted lines

> * Karl Wiberg <kha@treskal.com>:
>
> > On Wed, Dec 2, 2009 at 1:46 AM, Alex Chiang <achiang@hp.com> wrote:
> >
> > > +    for (p, n) in zip(patches, range(1, total_nr + 1)):
> > > +        msg_id = __send_message('patch', tmpl, options, p, n, total_nr, ref_id)
> >
> > Can be written as
> >
> >   for (n, p) in enumerate(patches):
> >
> > if you use n + 1 instead of n in the loop body.
>
> That is a little cleaner, but I decided to keep it as zip(). Why?
> Because using n + 1 in the loop body will push that line past 80
> columns. ;)
>
> It's also the original code (albeit with a simple variable rename).
>
> I know this isn't the kernel, and that there are plenty of other
> lines that are 80+ characters, but if you can keep it short, why
> not?

Oh, I fully favor keeping lines within the 80 columns allotted to us by the ancestors---I just didn't realize it was going to be a problem here.

In general, though, programmer time is worth optimizing for, and thinking through exactly what zip(patches, range(1, total_nr + 1)) means (and getting it right!) is a small but not insignificant cost every time someone reads the code.

-- 
Karl Wiberg, kha@treskal.com
   subrabbit.wordpress.com
   www.treskal.com/kalle