Ggthjy

I am currently using Python 3.5.2 to create a connect four basic game. My game should work for two players and tell you when you have won, be it horizontal vertical or diagonal. However I do not know how to make my code smaller or more efficient, is there a way to cut down, or is my method just not very efficient in itself?

I have tried looking up various other ways of doing this code, alas I found them difficult to understand. My code does not have any errors I have picked up on, and though I have completed the task I set myself, I do not feel this is the best way to do it.

If anybody had any suggestions, that would be much appreciated. If I knew how to make it shorter without losing functionality I would do so.

I hope my code will help anybody else attempting the same task as mine is rather basic.

import time    



grid1 = [0,0,0,0] # bottom row

grid2 = [0,0,0,0]

grid3 = [0,0,0,0]

grid4 = [0,0,0,0]



grids = [grid1,grid2,grid3,grid4]



check = []



user = 1



class fullSlot_error (Exception):

    pass

def hasWon_def():

    print ("player "+str(user)+" has won")

    time.sleep (1)



def grid_def():

    print ("",grid4,"n",grid3,"n",grid2,"n",grid1)



def user_def():

    global user

    if user < 2:

        user = 2

    else:

        user = 1

    return user



def slotFull_def():

   while True:

        try:

            if grid4[userInput -1] != 0:

                raise fullSlot_error

            else:

                break

        except fullSlot_error:

            print ("slot is full try again")

            confirm_def()



def confirm_def():

    looop= True

    while looop== True:

        try:

            global userInput

            userInput = int(input("ninput a slot player "+str(user)+"(1,4)n"))

            if userInput < 5 and 0 < userInput:   

                looop = False

            else:

                print ("invalid int")

        except ValueError:

            print ("invalid input")



def placement_def():

    counter = 0

    for i in range (0,4):

        slotFull_def()

        if (grids[i][userInput -1] == 0):

            grids [i][userInput - 1] = int(user)

            grid_def()

            break





def check_def():

    global loop

    global check

    for i in range(0,4):

        for a in range(0,4):

            check.append(grids[i][a])

        if (check == [1,1,1,1] or check == [2,2,2,2]):

            hasWon_def()

            loop = False

            return loop

            break

        else:

            check = []

    for i in range(0,4):

        for a in range(0,4):

            check.append(grids[a][i])

        if (check == [1,1,1,1] or check == [2,2,2,2]):

            hasWon_def()

            loop = False

            return loop

            break

        else:

            check = []



def checkEmpty_def():

    global check

    for i in range (0,4):

        for a in range (0,4):

            check.append(grids[i][a])

    if 0 not in check:

        print ("full")



def checks_def():

    check_def()

    checkEmpty_def()

    diagcheck_def()



def diagcheck_def():

    global loop

    global check

    check = []

    diag = 0

    for i in range (0,4):

        check.append (grids[diag][diag])

        diag = diag +1

        if (check == [1,1,1,1] or check == [2,2,2,2]):

            hasWon_def()

            loop = False

            return loop

            break

    check = []

    diag = 3

    diag2 = 0

    for i in range (0,4):

        check.append (grids[diag][diag2])

        if (check == [1,1,1,1] or check == [2,2,2,2]):

            hasWon_def()

            loop = False

            return loop

            break





loop = True



while loop == True:

    check_def()

    confirm_def()

    placement_def()

    checks_def()

    if loop == False:

        break

    user_def()

Some suggestions:

1. It is generally recommended to follow the pep8 style for Python code. For example no space between function names and paranthesis, lower_case_function_names, spaces after commas, etc.


2. You can define your grid in a single operation rather than 5.


3. For print, you can use * to unpack your nested list, then use sep='n' to put each row on a different line.


4. It would be better to have at least one top-level function, rather than doing everything in the root of the script.


5. You don't need global variables, it is better to pass the variables as arguments and return the result.


6. For print, if you give multiple values as arguments, spaces are automatically inserted between the arguments.


7. You don't get any benefit from using a custom exception here, just use a break in your if test.


8. you can use one-line if tests (called "ternary expressions") to simplify some of your code.


9. There isn't much benefit from a simple one-line function that is only used in one place. It makes things harder to read and slows the code down a tiny bit. Just inline the code.


10. There isn't much point using a sentinel variable (like looop) when you just assign False to it at a certain point. Just use break, and continue if need be to skip later parts of the loop.


11. It is better to put the absolute minimal amount of code possible in your try block to avoid accidentally catching the wrong exception.


12. you can combine multiple comparisons, such as 0 < userInput < 5.


13. You can use return instead of break to exit out of a while loop if you just want to return the result at that point with no further processing.


14. You can use in to test if a value is in a list or other sequence of values rather then using and.


15. You never use counter in placement_def.


16. The 0 is implied in range(0, x), so you can leave it out.


17. Rather than using range and indexing a list, it is easier to iterate over the list directly.


18. You always subtract 1 from userInput. It would be easier to subtract one at the beginning.


19. Rather than appending a list several times to another list, use extend to append the all the elements at once.


20. 
    check doesn't need to be global, its state is never shared between functions.


21. You can use all to simplify checking rows or columns, and zip to transpose lists.


22. You can use any and all to simplify your checks for whether a player has won and for whether the grid is full.


23. You could simplify the code somewhat by having the check functions return a boolean, then printing in checks_def.


24. I don't think placement_def is correct. You get a user input every time through the loop, rather than just once, and you overwrite the value if it is the current use or empty, when you want to overwrite only if empty. Further, you loop from top to bottom, when you want to loop from bottom to top.


25. You hard-code the grid size, but it would be easy to allow the user to specify a square grid size.

So here is my code:

def play(n=None):

    if n is None:

        while True:

            try:

                n = int(input('Input the grid size: '))

            except ValueError:

                print('Invalid input')

                continue

            if n <= 0:

                print('Invalid input')

                continue

            break



    grids = [[0]*n for _ in range(n)]

    user = 1

    print('Current board:')

    print(*grids, sep='n')

    while True:

        user_input = get_input(user, grids, n)

        place_piece(user_input, user, grids)

        print('Current board:')

        print(*grids, sep='n')



        if (check_won(grids, user, n) or

                check_won(zip(*grids), user, n) or

                diagcheck_won(grids, user, n) or

                diagcheck_won(grids[::-1], user, n)):

            print('Player', user, 'has won')

            return



        if not any(0 in grid for grid in grids):

            return



        user = 2 if user == 1 else 1





def get_input(user, grids, n):

    instr = 'Input a slot player {0} from 1 to {1}: '.format(user, n)

    while True:

        try:

            user_input = int(input(instr))

        except ValueError:

            print('invalid input:', user_input)

            continue

        if 0 > user_input or user_input > n+1:

            print('invalid input:', user_input)

        elif grids[0][user_input-1] != 0:

            print('slot', user_input, 'is full try again')

        else:

            return user_input-1





def place_piece(user_input, user, grids):

    for grid in grids[::-1]:

        if not grid[user_input]:

            grid[user_input] = user

            return





def check_won(grids, user, n):

    return any(all(cell == user for cell in grid) for grid in grids)





def diagcheck_won(grids, user, n):

    return all(grids[x][x] == user for x in range(n))





if __name__ == '__main__':

    play()

Note that this could be simplified even further using numpy arrays, but that is more advanced.