Nested For and While Statements

Paul Hankin paul.hankin at gmail.com
Mon Sep 24 18:02:48 EDT 2007


First the bugs...

On Sep 24, 9:52 pm, kou... at hotmail.com wrote:
> ...
>     avcsasdoc = workspace + "\avcsas.txt"
> ...

There's quite a few cases like this where you don't escape
backslashes, so your filenames are mostly mangled. Here \a produces
character code 7, which is definitely wrong.

Either escape them like this: '\\avcsas.txt' or use raw strings where
backslashes aren't treated specially: r'\avcsas.txt'. You might want
to look at the 'os.path' module for help in constructing filenames.

You have a 'break' and the end of the last for loop. This has the
effect of stopping the for loop at the first iteration. Either it's a
bug, or you really didn't want a for loop!


There's also quite a few places where your code could be improved:

>         classsum = avcclass[0] + avcclass[1] + avcclass[2] +
> avcclass[3] + avcclass[4] + avcclass[5] \
>        + avcclass[6] + avcclass[7] + avcclass[8] + avcclass[9] +
> avcclass[10] \
>        + avcclass[11] + avcclass[12] + avcclass[13]

Is better written
     classsum = ''.join(avcclass[:14])


> addzeros = {1:"0", 2:"00", 3:"000", 4:"0000"}
> ...
> ... addzeros[numzeros] + lanex[i-1]

Good idea, but in this case python provides an easier alternative:
  ... '0' * numzeros + lanex[i - 1]
Or use the 'rjust' method which right-justifies a string:
  ... lanex[i - 1].rjust(5, '0')


> for i in range(1,Nscheme+1):
>      numzeros = 5 - int(len(lanex[i-1]))
>      avcclass.append(addzeros[numzeros] + lanex[i-1])
>      Nlanex.append(int(lanex[i-1])

Is better as
  for i in range(Nscheme):
      ... use i rather than i - 1

(I assume Visual Basic for loops are off by one).

In fact, this loop is even better if you just iterate over lanex
itself. Using a loop index rather than iterating directly is a common
mistake.

for lane in lanex:
    avcclass.append(lane.rjust(5, '0'))
    Nlanex.append(int(lane))

Or even better, use list comprehensions to construct your two lists
explicitly:

avcclass = [lane.rjust(5, '0') for lane in lanex]
Nlanex = [int(lane) for lane in lanex]

Note, each rewrite makes the intention of the code clearer, as well as
being shorter. This makes your code easier to read and understand, and
less buggy.


Since all you do with avcclass is sum it, combine this with the
classsum optimisation above to get

classsum = ''.join(lane.rjust(5, '0') for lane in lanex[:14])
Nlanex = [int(lane) for lane in lanex]

>    lanex = []
>    if numlanes == 2:
>        lane1 = newlist1[2:2+Nscheme]
>        lanex.append(lane1)
>        lane2 = newlist1[2+Nscheme:2+Nscheme*2]
>        lanex.append(lane2)
>    else:
>        lane1 = newlist1[2:2+Nscheme]
>        lanex.append(lane1)
>        lane2 = newlist1[2+Nscheme:2+Nscheme*2]
>        lanex.append(lane2)
>        lane3 = newlist1[2+Nscheme*2:2+Nscheme*3]
>        lanex.append(lane3)
>        lane4 = newlist1[2+Nscheme*3:2+Nscheme*4]
>        lanex.append(lane4)

List comprehensions beat copy/paste:

total_schemes = 2 if numlanes == 2 else 4
lanex = [newlist1[2 + i * Nscheme:2 + (i + 1) * Nscheme]
    for i in range(total_schemes)]

HTH, and welcome to python!

--
Paul Hankin




More information about the Python-list mailing list