Log in

No account? Create an account

Fri, Oct. 16th, 2009, 10:26 am

This post unintentionally demonstrates that functional-style Python is ugly and bad.

Let us start at the top. At the beginning, it has some sample code which defines a multiple() function, which could be trivially inlined, resulting in code which looks like this (I'm doing all examples in Python 3):
print(sum(x for x in range(1, 1000+1) if x % 3 == 0 or x % 5 == 0))

There's no reason whatsoever to expand that out. This should be an early indication that maybe the code samples here aren't the greatest.

Moving on, there's getting the sum of all fibonacci numbers less than 4 million. This is done in the example using itertools and yield, resulting in a fair amount of ugly code. Here's how a sane person does it:
def fibsum():
	a, b, c = 0, 1, 1
	total = 0
	while c < 4000000:
		total += c
		a, b, c = b, c, b + c
	return total

Now that's much more readable, flexible, and maintainable.

Finally, there's the problem of finding the largest palindrome which is the product of two three digit numbers. Here's my solution, which contains less code, is much more readable, and oh yeah, I threw in an optimization to make it return almost instantly instead of having to crunch for a second:
def bigpalindrome():
	best = 0
	for i in range(999, 0, -1):
		if i * 999 < best:
			return best
		for j in range(999, i-1, -1):
			x = list(str(i*j))
			if int(''.join(x)) == i*j:
				best = i*j

Much better. I think these examples do a good job of exploding the idea that the functional style of programming is clearly better and the appropriate first thing to teach people. Obviously some people are being driven to write horribly contorted and ugly code because they started in a functional language when they switch a more, ahem, mainstream one.

Fri, Oct. 16th, 2009 10:22 pm (UTC)

FWIW, here's how I'd do the problem. It runs slower, but that's OK; this is my default style because it mirrors the problem statement more directly. (And I guess because I used to use Scheme a lot.) I glanced at the code you linked to, and yeah, convoluted.

def bigpalindrome():
    return max(n for n in products() if is_palindrome(n))

def products():
    return (i*j for i in xrange(100, 1000) for j in xrange(i, 1000))

def is_palindrome(n):
    s = str(n)
    return s[::-1] == s

Fri, Oct. 16th, 2009 11:21 pm (UTC)

Your comparison method of str(n)[::-1] == str(n) is terser and probably more idiomatic than mine, but you have a bunch of bugs in your products() function, which should be written using yield instead of comprehensions.

Sat, Oct. 17th, 2009 12:18 am (UTC)

I realize products() computes a different set of candidates than you did; I went by your English and not your code, taking 42 to not be a 3-digit number. Do you mean something else?

I'd still move the str() in my code to the caller of is_palindrome() -- there's always one more tweak.

Sat, Oct. 17th, 2009 12:35 am (UTC)

Turns out max() is a lot more sophisticated than I thought, so there was actually nothing wrong with your code.