This is my first project in pygame so I am open to suggestions how I can improve my python/pygame skills. I have started programming in python couple of weeks ago and I really want to know what I am doing wrong/harder way.

Main game file

import pygame

from time import sleep

from square import square, superior

import copy



pygame.init()



#Colors

background = (0, 0, 0)

deadCell = (195, 200, 181)

aliveCell = (0, 27, 9)



#Can change those if you want

amount = 20

size = 20



#Some math to calculate size of the screen based on size and amount of 

cells + space between them

display_size = ((amount*size) + amount - 1, (amount*size) + amount - 1) 

display = pygame.display.set_mode(display_size)



exitGame = False

gameStarted = False



grid = list()



#FPS control variables

FPS = 2

clock = pygame.time.Clock()



def create_array():

    global grid



    for row in range(0, amount):

        placeholder = 

        for column in range(0, amount):

            cell = square(row, column, column*size + column, row*size + row, amount, size)

            placeholder.append(cell)

        grid.append(placeholder)



def create_grid():

    display.fill(background)

    #Render grid

    for row in grid:

        for cell in row:

            if cell.alive:

                pygame.draw.rect(display, aliveCell, [cell.x, cell.y, size, size])

            else:

            pygame.draw.rect(display, deadCell, [cell.x, cell.y, size, size])

pygame.display.update()



def check():

    global grid



    #Create old gird tamplate

    old_grid = copy.deepcopy(grid)



    for row in grid:

        for cell in row:

            alive = 0



            next_row = cell.row + 1

            previous_row = cell.row - 1



            next_column = cell.column + 1

            previous_column = cell.column - 1



            # To prevent calling old_grid[-1]

            if previous_row < 0:

                previous_row = len(row) + 100



            if previous_column < 0:

                previous_column = len(row) + 100



            #Check for alive neighbors

            try:

                if old_grid[next_row][cell.column].alive:

                    alive += 1

            except:

                pass

            try:

                if old_grid[previous_row][cell.column].alive:

                    alive += 1

            except:

                pass

            try:

                if old_grid[cell.row][previous_column].alive:

                    alive += 1

            except:

                pass

            try:

                if old_grid[cell.row][next_column].alive:

                    alive += 1

            except:

                pass

            try:

                if old_grid[previous_row][previous_column].alive:

                    alive += 1

            except:

                pass

            try:

                if old_grid[previous_row][next_column].alive:

                    alive += 1

            except:

                pass

            try:

                if old_grid[next_row][previous_column].alive:

                    alive += 1

            except:

                pass

            try:

                if old_grid[next_row][next_column].alive:

                    alive += 1

            except:

                pass



            if alive < 2 or alive > 3:

                cell.alive = False

            elif alive == 3:

                cell.alive = True



#Initialize grid list and render first grid view

create_array()

create_grid()



while not exitGame:



    for event in pygame.event.get():

        #Check if user wants to quit

        if event.type == pygame.QUIT:

            exitGame = True



        #Placing cells

        if event.type == pygame.MOUSEBUTTONDOWN and not gameStarted:

            for row in grid:

                for cell in row:

                    click = pygame.mouse.get_pos()

                    if cell.is_in_range(click[0], click[1]):

                        cell.alive = (not cell.alive)



        #Start the game

        if event.type == pygame.KEYDOWN:

            if event.key == pygame.K_SPACE:

                gameStarted = True



    if gameStarted: 

        check()



        #Check if all cells are dead

        if superior().number_alive == 0:

            exitGame = True



    #Render grid

    create_grid()   

    clock.tick(FPS)





pygame.quit()

quit()

Square.py file

class superior():

    number = 0

    number_alive = 0



class square(superior):



    number = 0



    def __init__(self, row, column, x, y, number_of_columns, size):

        self.row = row

        self.column = column

        self.x = x

        self.y = y

        self.number_of_columns = number_of_columns

        self.size = size

        self.live = False



        superior.number += 1



    def is_in_range(self, x, y):

        if x in range(self.x, self.x + self.size + 1) and y in range(self.y, self.y + self.size + 1):

            return True

        else:

            return False



    @property

    def alive(self):

        return self.live



    @alive.setter

    def alive(self, isalive):

        if self.live and not isalive:

            superior.number_alive -= 1

        elif not self.live and isalive:

            superior.number_alive += 1



        self.live = isalive

I've never used pygame, but I can make suggestions regarding the rest of your code.

In your main file, you have two functions, both of which contain the line global grid at the top. Since some external grid is a dependency of the function, it should be explicitly passed in as a parameter instead:

global_grid = 



def create_array(grid):

    for row in range(0, amount):

    ...



create_array(global_grid)

Why? Now create_array can be tested entirely independently of anything else going on. You can create some test grid to do testing with in a REPL, pass the grid to create_array, and have it do its thing. With how you have things set up now though, to do any testing you need to actually alter the code by changing the global grid.

If you look at create_array, why does it even need to know about grid though? All it's doing is appending data to it; and it seems likely that it's assuming grid is already empty. It would make much more sense to have something like this where it just has its own grid:

# Arguably, "amount" should be a parameter as well

def create_array():

    grid =  # No more global reference, instead create a local



    for row in range(0, amount):

        placeholder =  # "row# may be a more appopriate name

        for column in range(0, amount):

            cell = square(row, column, column*size + column, row*size + row, amount, size)

            placeholder.append(cell)

        grid.append(placeholder)



    return grid # Now return the local



global_grid = create_array() # Assign the return of the function

Ideally, each function should explicitly take as parameters any data that it needs to operate, and explicitly return any data that represents its purpose. With this new version, the function can be tested by running it and examining the output in the REPL. You aren't needlessly altering any global data that other function calls later on may be effected by.

And the same goes for check. What if you want to test its checking ability by throwing random data at it? If all your functions rely on the same global, and you're altering that global for every test, you're also potentially changing the behavior of any other function that's relying on the global grid. Change it to:

def check(grid):

    #Create old gird tamplate

    old_grid = copy.deepcopy(grid)

    ...

And pass in the grid you want checked when you want to call it.

I'll just mention as an aside that the expensive copy.deepcopy(grid) call can be entirely avoided if you use immutable data structures. This is a prime case where immutability is superior. In languages like Clojure and Haskell that use immutable structures as the default, creating copies of data is very cheap. When I write a Conway's Game of Life implementation in Clojure, I never have to manually create a deep copy of the grid, because every time I "alter" the grid, I'm making a cheap copy of it automatically. Unfortunately, I don't think Python has very good support for immutable data structures (unless a good library is available, which I wouldn't doubt actually). It's something to keep in mind though when working on projects like this.

try:

    if old_grid[cell.row][previous_column].alive:

        alive += 1

except:

    pass

This is literally one of the worst things you could possibly do. In this particular case, only a limited number of things can go wrong, but you are setting yourself up for excruciating debugging if you completely throw away any error you may receive.

Let's say in the future you add a function call in there, but you haven't sufficiently tested the new function. One day, it breaks due to some unforeseen circumstance, and throws an exception. Normally, the exception would cause the program to crash, it tells you what's wrong, you read the error and fix the problem, and get it working again. With how you have it now, try...except: pass will silence any error that you receive; regardless of its source or reason. As someone that's been bit by exception swallowing before (via Clojure's futures), I can tell you it is the worst debugging and programming experiences you can have. Everything suddenly stops working in weird ways, yet you have no errors indicating what's wrong. It's awful. Don't do that to yourself.

I'm assuming you're doing this to simplify out of bounds checks. Using exceptions to handle bounds checks will likely be slower that the alternative of just not checking out of bounds cells in the first place. I'm going into brain lock on how to better approach this in Python, but iterating over the indices of the grid and using the min and max functions that account for the width/height of the grid would be better. You could also create a is_inbounds function that returns if a given coordinate is valid in the grid or not, and use the check.

At the very least, if you wanted to keep using the "ask for forgiveness instead of permission" approach, you should specify what type of exception you're catching:

try:

    if old_grid[cell.row][previous_column].alive:

        alive += 1



except IndexError as e: # Specify that you only want to catch IndexErrors

    pass

def is_in_range(self, x, y):

  if x in range(self.x, self.x + self.size + 1) and y in range(self.y, self.y + self.size + 1):

    return True

  else:

    return False

Would be more succinctly written as

def is_in_range(self, x, y):

  return x in range(self.x, self.x + self.size + 1) and

         y in range(self.y, self.y + self.size + 1)

Or, to take advantage of Python allowing comparison chaining:

def is_in_range(self, x, y):

  return (self.x <= x <= self.size) and

         (self.y <= y <= self.size)

if event.type == pygame.KEYDOWN:

    if event.key == pygame.K_SPACE:

        gameStarted = True

Is the same as:

if event.type == pygame.KEYDOWN and event.key == pygame.K_SPACE:

    gameStarted = True

If you had an elif or else after one of the condtions and needed to handle false cases, it would be different. Here though, you only care if they're both true (and).

Those are the major things that stuck out to me. I hope that helped.