4
\$\begingroup\$

I have this interpretation of John Conway's Game of Life in Python 3.3.1:

#JOHN CONWAY'S GAME OF LIFE
def countSurrounding(universe, a, b):
    count = 0
    surrounding = [[a - 1, b - 1],
                   [a - 1, b    ],
                   [a - 1, b + 1],
                   [a    , b - 1],
                   [a    , b + 1],
                   [a + 1, b - 1],
                   [a + 1, b    ],
                   [a + 1, b + 1]]
    for a in range(0, 8):
        try:
            if universe[surrounding[a][0]][surrounding[a][1]] == 1:
                count += 1
        except IndexError: a
    return count
def printUniverse(universe):
    for a in universe:
        print(a)
    print("\n")
nextUniverse = [[0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 1, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 1, 0, 0, 0, 0],
                [0, 0, 1, 1, 1, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0]]
for gen in range(0, 10):
    universe = [a[:] for a in nextUniverse]
    printUniverse(universe)
    for a in range(0, 9):
        for b in range(0, 9):
            if universe[a][b] == 0 and countSurrounding(universe, a, b) == 3:
                nextUniverse[a][b] = 1
            elif universe[a][b] == 1 and countSurrounding(universe, a, b) not in (2, 3):
                nextUniverse[a][b] = 0

How can I improve/optimize my code?

EDIT: After the answers below, I have updated my code:

#JOHN CONWAY'S GAME OF LIFE
def countSurrounding(universe, a, b):
    count = 0
    surrounding = ((a - 1, b - 1),
                   (a - 1, b    ),
                   (a - 1, b + 1),
                   (a    , b - 1),
                   (a    , b + 1),
                   (a + 1, b - 1),
                   (a + 1, b    ),
                   (a + 1, b + 1))
    for a, b in surrounding:
        if not(a < 0 or b < 0 or a >= len(universe) or b >= len(universe[a])) and universe[a][b]:
            count += 1
    return count
def printUniverse(universe):
    for a in universe:
        print(a)
    print("\n")
nextUniverse = [[0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 1, 0, 0, 0, 0, 0],
                [0, 0, 0, 1, 0, 0, 0, 0, 0],
                [0, 0, 0, 1, 0, 0, 0, 0, 0],
                [0, 0, 0, 1, 0, 0, 0, 0, 0],
                [0, 0, 0, 1, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0],
                [0, 0, 0, 0, 0, 0, 0, 0, 0]]
for gen in range(0, 10):
    universe = [a[:] for a in nextUniverse]
    printUniverse(universe)
    for a in range(0, len(universe)):
        for b in range(0, len(universe[a])):
            if universe[a][b] == 0 and countSurrounding(universe, a, b) == 3:
                nextUniverse[a][b] = 1
            elif universe[a][b] == 1 and countSurrounding(universe, a, b) not in (2, 3):
                nextUniverse[a][b] = 0

Any more improvements to be made? So far I have:

  • Introduced a check in countSurrounding to replace the except IndexError, so that the board does not wrap.
  • Made surrounding a tuple instead of a list
  • Changed for a in range(0, 9) to for a in range(0, len(universe)), as I assume that is more pythonic
\$\endgroup\$
4
\$\begingroup\$

Overall, it seems really really good to me and I like the way you use IndexError to count surrounding cells in a concise way without bothering about edge cases.

Here are a few comments anyway :

Removing a useless check

This is not much but you can remove a check of universe[a][b] in

        if universe[a][b] == 0 and countSurrounding(universe, a, b) == 3:
            nextUniverse[a][b] = 1
        elif universe[a][b] == 1 and countSurrounding(universe, a, b) not in (2, 3):
            nextUniverse[a][b] = 0

and it becomes

        surr = countSurrounding(universe, a, b)
        if universe[a][b]:
            if surr not in (2, 3):
                nextUniverse[a][b] = 0
        else:
            if surr == 3:
                nextUniverse[a][b] = 1

(It looks slightly longer though) (I also took this chance to use an additional variable)

Use tuple instead of list if it's not something you plan to iterate over

Here, the surroundings are just pairs and that should be enough. Also, it makes the access of the different elements much more convenient through tuple unpacking.

Iterate in a pythonic way

You shouldn't use range to iterate over a structure when python provide such a convenient way to iterate over it.

For instance, you could do :

surrounding = [(a - 1, b - 1),
               (a - 1, b    ),
               (a - 1, b + 1),
               (a    , b - 1),
               (a    , b + 1),
               (a + 1, b - 1),
               (a + 1, b    ),
               (a + 1, b + 1),]
for a,b in surrounding:
    try:
        if universe[a][b]:
            count += 1
    except IndexError: a

Same kind of argument applies to for a in range(0, 9): for b in range(0, 9): even though you might want to introduce enumerate.

\$\endgroup\$
  • 1
    \$\begingroup\$ One can also iterate over a tuple, and unpack a list. \$\endgroup\$ – Janne Karila May 3 '13 at 18:06
  • \$\begingroup\$ Even though I completely agree, it doesn't make my comment invalid, does it ? (I try to follow the idea of "loopy lists and structy tuples") \$\endgroup\$ – Josay May 7 '13 at 14:37
4
\$\begingroup\$

There is a problem with your IndexError technique.

Negative indexes will wrap around the board, whereas indexes which are too large will cause IndexError. This means that you'll have a wraparound happening on one side, not the other.

For more speed, you should look into the numpy library. It'll allow you to hold the game board in an array and do vector operations which will be much faster.

\$\endgroup\$
  • \$\begingroup\$ can you explain more about numpy vector operation? \$\endgroup\$ – inyoot Jun 10 '15 at 22:16
0
\$\begingroup\$

i find it confusing that inside countSurrounding, 'a' stands for both a function parameter, and an index variable. Besides that, it looks good

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.