Please Criticize My Code

Michael Hoffman cam.ac.uk at mh391.invalid
Sat Aug 20 04:52:05 EDT 2005


Ray wrote:

> I just wrote a short script that generates look and say sequence. What
> do you Python guys think of it? Too "Java-ish"?

Yes, but that's beside the point. :)

I think your basic design was sound enough for this application 
(presumably this isn't something that needs to run at high speed). I 
found it a little hard to understand what was going on. Part of this was 
due to variable and function naming choices. Converting between strs and 
ints all the time is confusing too.

I think this version might be a little easier to understand, and 
describe() should work for any string, not just integers. I think your 
original version does as well:

def describe(string):
     last_char = ""
     last_count = ""
     res = []

     for char in string:
         if char == last_char:
             last_count += 1
         else:
             res.extend([str(last_count), last_char])
             last_char = char
             last_count = 1

     res.extend([str(last_count), last_char])

     return "".join(res)

def describe_generator(start, count):
     string = str(start)

     for index in xrange(count):
         yield string
         string = describe(string)

print list(describe_generator(1, 30))

As requested, here's a more concise, but difficult to understand version 
of describe() using regexes:

import re

re_describe = re.compile(r"(.)\1*")
def describe(string):
     runs = re_describe.finditer(str(string))

     return "".join("".join([str(len(run.group(0))), run.group(0)[0]])
                    for run in runs)

While it is fewer lines of code, it's not as easy to see what this 
*thing* does immediately upon looking at it. So I'd try to avoid this 
version, really.
-- 
Michael Hoffman



More information about the Python-list mailing list