Day: July 30, 2012

Long time no blog. I’d like to break the ice with a lighthearted Python code readability optimization discussion.

I need a Python function that generates short, random, numeric codes of a specified length using a secure random number generator. The codes must not start with 0, but the output should be a string. This was my first draft:

import struct, os

def make_code(digits):
    """Generate a random short code."""
    if digits > 9:
        return make_code(9) + make_code(digits - 9)
    [rand] = struct.unpack('>L', os.urandom(4))
    min_code = 10 ** (digits - 1)
    code = min_code + rand % (9 * min_code)
    return unicode(code)

Two things about this solution bothered me:

• The math I used looks too clever. I’m not sure future maintainers will be able to understand why it works.
• It discards a lot of the entropy provided by /dev/urandom, which could be expensive on some servers. (I confess this thought is premature optimization.)

To try to clean this up, I created a generator that produces random secure digits and based the make_code function on it:

def random_digits():
    while True:
        [n] = struct.unpack('>L', os.urandom(4))
        s = ('%d' % n)[1:]
        for c in s:
            yield c

next_random_digit = random_digits().next

def make_code(digits):
    return ''.join(next_random_digit() for _ in range(digits))

This felt better, but it has two critical problems. It can return 0 for the first digit, and due to the next_random_digit global, this solution is not thread safe! I’ve never tried to run a generator in multiple concurrent threads. I can only imagine the possible fireworks.

So I solved the problems as follows.

class RandomDigitStream(object):
    def __init__(self):
        self.streams = threading.local()

    def stream(self):
        while True:
            [n] = struct.unpack('>L', os.urandom(4))
            s = ('%d' % n)[1:]
            for c in s:
                yield c

    def make_code(self, digits):
        """Generate a random short code containing only digits."""
        try:
            next_digit = self.streams.next_digit
        except AttributeError:
            self.streams.next_digit = next_digit = self.stream().next

        while True:
            c = next_digit()
            # Don't use 0 for the first digit.
            if c != '0':
                break
        lst = [c]
        lst.extend(next_digit() for _ in range(1, digits))
        return ''.join(lst)

make_code = RandomDigitStream().make_code

Now the solution is large and I’m pretty sure it’s less readable than the original solution. Anyone want to take a shot at making this better? I’m currently thinking that the original solution was better than I thought.