Beginner's pygame Conway's Game of Life

Here's my implementation of Game of Life. I'm quite new to Python and would like to know how could I improve that code, especially in terms of performance, compactness and readability.

```
import copy
import pygame
from pygame import *
import random

#constants
red = (255, 0, 0)
black = (0, 0, 0)
white = (255, 255, 255)
#neighbour coordinates
neighbours = [[-1,-1],[-1,0],[-1,+1],
[0,-1], [0,+1],
[+1,-1],[+1,0],[+1,+1],]

class cell(object):
def __init__(self, ngb, state):
self.state = state
self.ngb = 0

#2d array for storing cells
cells = [[i for i in range(50)] for i in range(50)]

#random field generation
def generate():
print "Generating"
for y in xrange(50):
for x in xrange(50):
cells[x][y] = cell(0, random.randint(0, 1))
print "DoneGen"

#neighbour processing
def update():
global cells2
#saving this turn's state
cells2=copy.deepcopy(cells)
for y in xrange(50):
for x in xrange(50):
cellv2=cells2[x][y]
cellv2.ngb=0
cellv = cells[x][y]
#processing
for i in neighbours:
#offsetting neighbour coordinates
dy=i[0]+y
dx=i[1]+x
if dy 49:
dy = 0
if dx 49:
dx = 0
if cells2[dx][dy].state==1:
cellv2.ngb+=1
#updating field
if cellv2.state==1 and 2<=cellv2.ngb<=3:
cellv.state=1
else:
cellv.state=0
if cell

Functions good practices: Documentation, Parametrization, Single Purpose, Outside State Independence

Documentation

#random field generation
def generate():

Documentation is usually written as a docstring in triple quotes under the function definition (this allows programmatic access via help:

def generate():
    """ Random field generation. """

It also usually is a complete phrase, I would write it a little different:

def generate():
    """ Generates a random game of life board. """

Parametrization

Your function can only generate boards of size \$50 * 50\$, may I be interested in other sizes I would need to modify the definition accordingly.

I suggest asking x and y sizes as parameters:

def generate(x_size, y_size):
        print "Generating"
        for y in xrange(x_size):
                for x in xrange(y_size):
                        cells[x][y] = cell(0, random.randint(0, 1))
        print "DoneGen"

This way you gain more re-usability for your function (and you can test it simpler by giving small sizes).

Single Purpose

print "Generating"
    ...
    print "DoneGen"

Your function prints to std-out in addition to building the board and this behaviour cannot be turned off. If printing out is not desired the user will not be able to use this function. I would just delete these printing statements after the debugging is complete.

Outside state independence

You need a 2d list called board for this function to work. You may instead create such list and return it.

Write functions to encapsulate logical units of action

wrap_around(dx, dy, x_size, y_size)

if dy  49:
                                    dy = 0
                            if dx  49:
                                    dx = 0

This 8 lines of code provide the program with wrap-around functionality (for example: going too far to the right leaves you all the way back to the left), it would ideal if you wrote a function for this.

next_state(cell, neighbours)

The same can be said for this block of code calculates the next state of a given cell:

if cellv2.state==1 and 2<=cellv2.ngb<=3:
                            cellv.state=1
                    else:
                            cellv.state=0
                    if cellv2.state==0 and cellv2.ngb==3:
                            cellv.state=1

decide_colour(cell)

if cells[x][y].state==1:
                                    pygame.draw.rect(mainsrf, black, (x*10, y*10, 10, 10))
                            else:
                                    pygame.draw.rect(mainsrf, white, (x*10, y*10, 10, 10))
                            if cells[x][y].ngb==3:
                                    pygame.draw.rect(mainsrf, red, (x*10, y*10, 10, 10))

The 3 options differ only in the colour of the drawing so you could also cut down code duplication by using such a function.

Minor: use ALL_CAPS for constants

As a widely accepted convention constants are written ALL CAPS to quickly discern them from variable variables.

RED = (255, 0, 0)
BLACK = (0, 0, 0)
WHITE = (255, 255, 255)
#neighbour coordinates
NEIGHBOURS = [[-1,-1],[-1,0],[-1,+1],
              [0,-1],        [0,+1],   
              [+1,-1],[+1,0],[+1,+1],]

Give meaningful names: ngb?

self.ngb = 0

I cannot understand what the purpose of the ngb field is even after reading the code.

This is a bad sign indicating that a more descriptive name should be adopted.

StackExchange Code Review Q#131689, answer score: 5