Please Criticize My Code

[Ray]

Hello,

I just wrote a short script that generates look and say sequence. What
do you Python guys think of it? Too "Java-ish"? I can't shake off the
feeling that somebody may have done this with 2-3 lines of Python
magic. Heh.

# generator for sequence
def lookAndSaySequence(firstTerm, n):
term = str(firstTerm)
for i in xrange(n):
yield term
term = lookAndSay(term)

# the method that looks, and says
def lookAndSay(number):
look_at_this = str(number)
say_this = ['0', look_at_this[0]]
for digit in look_at_this:
if say_this[-1] != digit:
say_this.extend(('1', digit))
else:
say_this[-2] = str(int(say_this[-2]) + 1)
return "".join(say_this)

# run it!
print [x for x in lookAndSaySequence(1, 30)]

Thanks,
Ray

 

[Michael Hoffman]

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.

 

[bearophileHUGS]

Two versions of mine, one of the fastest (not using Psyco) and one of
the shortest:

.. from itertools import groupby
..
.. def audioactiveFast(n):
.. strl = {("1","1","1"): "31", ("1","1"): "21", ("1",): "11",
.. ("2","2","2"): "32", ("2","2"): "22", ("2",): "12",
.. ("3","3","3"): "33", ("3","3"): "23", ("3",): "13" }
.. result = [1]
.. prec = "1"
.. for i in xrange(n-1):
.. prec = "".join( strl[tuple(l)] for e,l in groupby(prec) )
.. result.append( int(prec) )
.. return result
..
..
.. def audioactiveShort(n):
.. s = [1]
.. for i in xrange(n-1):
.. r = 0
.. for e,l in groupby(str(s[-1])):
.. r = r*100 + len(list(l))*10 + int(e)
.. s.append( r )
.. return s

Bye,
bearophile

 

[Ray]

Damn, those are cool, man. Thanks! This Python thing keeps expanding
and expanding my brain...

Ray

(e-mail address removed) wrote:
<snipped>

 

[Ray]

Yes, but that's beside the point.

Well... I'm always paranoid that I'm, you know, writing Java in Python


<snipped>

Thanks for the examples! That last one took me a while to understand. I
like the way you approach it though (e.g.: going after clarity first
instead of shortness).

Cheers
Ray

 

[Michael Hoffman]

Well... I'm always paranoid that I'm, you know, writing Java in Python

The biggest mistakes I see are wrapping everything in a class
unnecessarily, or using accessor methods instead of properties (ugh,
ugh, ugh!). CamelCasingYourVariableNames is annoyingToMe but that's
really a stylistic choice.

Thanks for the examples! That last one took me a while to understand. I
like the way you approach it though (e.g.: going after clarity first
instead of shortness).

I think that is the Pythonic way. <wink> In general, I find conciseness
in code is not worth all that much in its own right. Although sometimes
it can make things easier to understand, which is good. There is a
common belief that concise code = fast code, and it is easy to
demonstrate that this is false. See, for example, this essay:
<http://www.python.org/doc/essays/list2str.html>

Also, if you haven't done so already, start up an interactive prompt and
type "import this." Key to consider:

Simple is better than complex.
Complex is better than complicated.
Sparse is better than dense.
Readability counts.
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.

Best regards,

 

[Christoph Rackwitz]

i guess, it is pythonchallenge.com level 10?
if so, i used this thing:

import re
def enc(s):
return ''.join('%s%s' % (len(a[0]),a[0][0]) for a in
re.findall('((.)\\2*)', s))

 

[Jeff Schwab]

i guess, it is pythonchallenge.com level 10?
if so, i used this thing:

import re
def enc(s):
return ''.join('%s%s' % (len(a[0]),a[0][0]) for a in
re.findall('((.)\\2*)', s))

Don't do that!

 

[Christoph Rackwitz]

Why not? Because the regex isn't compiled?
Don't tell me not to do something, tell me why i should'nt do it.

 

[Ray]

Why not? Because the regex isn't compiled?
Don't tell me not to do something, tell me why i should'nt do it.

No it's not the regex, it's because you just spoiled the challenge for
everybody who hasn't solved level 10 yet...