HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

Connect 4 refine the diagonal check

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
refinetheconnectdiagonalcheck

Problem

I am creating a connect 4 game in python and I have managed to write the code so that it runs. However I have a lot of duplicated data in it and was just wondering if anyone could help me to refine it to minimise this?

Here is my code so far:

```
def CreateBoard(r, c, b, n):

for i in range(1,(r+1)):
n.append(str(i))
b.append(n)

for i in range(c):
b.append([''](r))
return(b)

"""The Following PrintBoard function takes the parameter board(b), prints
the board and then returns the board."""

def printBoard(b):
for lst in b:
print(" ".join(lst))
return b

"""The following Check function takes the parameter board. Within this """
def check(board, n):
n = []
for i in range(1,len(board[1])+1):
if board[1][i-1] == '*':
n.append(i)
print(n)

user = (input('Enter column: '))

if(user.isdigit() == True):
user = int(user)
if (user in n):
return(user)
print('Invalid input')
return check(board, n)

def WinningCon(b, r, u, c):
loop1 = True
rowCon = ""
colCon = ""
for i in range(0,len(b[1])):
rowCon += b[r][i]
if('X'4 in rowCon) or('O'4 in rowCon):
return(True)
for i in range(1,c+1):
colCon += b[i][u-1]
if('X'4 in colCon) or('O'4 in colCon):
return(True)

def Diag2(u, r, b, column, row):
utemp1 = u-1
utemp2 = u-1
rtemp1 = r
rtemp2 = r
end = ""
beg = ""
while(True):

beg += b[rtemp1][utemp1]
rtemp1 -= 1
utemp1 += 1
if(rtemp1 == 1):
break
elif(utemp1 >= column - 1):
break
while(True):
end += b[rtemp2][utemp2]
rtemp2 +=1
utemp2 -=1
if(rtemp2 >= row):
break
elif(utemp2 == 0):
break
beg = beg[::-1]
fullstring = beg+end
if('X'4 in fullstring) or('O'4 in fullstring):
return(True)

def Diag1(u, r, b, column, row):
utemp1 = u-1
utemp2 = u-1
rtemp1 = r
rtemp2 = r
end = ""
beg = ""
while(True):
beg += b[rtemp1][utemp1]
rtemp1 -= 1
utemp1 -= 1
if(rtemp1 == 1):

Solution

Severe code readability issues

Honestly, your code is extremely difficult to follow and I gave up. The most important part about code is readability. Readable code is easier to reason about, easier to maintain, easier to debug, and, especially relevant here, easier to get other people's opinions on. I would very much like to give you advice on how to improve your functions - but I can't. I have no idea what they're doing now. They're simply inscrutable.

It also isn't enough that you understand what this code does right now. What if you need to go back to this code in a week? In 3 months? In a year? Will you remember what it does then? Even well-commented code with good names sometimes becomes hard to understand once you forget the context of when it was written. Poorly named, uncommented code is just setting yourself up for lots of confusion and frustration. At best.

I cannot stress this enough. Read up on the python style guide, especially for its conventions for naming and comments.

As far as examples of the above. We have:

def CreateBoard(r, c, b, n):


What are r, c, b, and n? I can't even tell from the body of the function. Avoid single-letter variable names unless used in a context where they are truly trivial (e.g. iterating over a range with i or _ is fine).

Function names should be snake_case, not CamelCase, and should describe what the function actually does. Certainly Diag1 and Diag2 are not helpful at all. Which one checks which kind of diagonal? What arguments do they take? What do they return?

docstrings belong inside the function:

def print_board(b):   
    """This function takes the parameter board(b), prints
    the board and then returns the board."""


When you need a comment to describe what your variable name names, that's a good indicator that you should just change your variable name. Once you do that, that doc string becomes pointless:

def print_board(board):
    ...


It's pretty clear that a function called print_board that takes a board will print it. And if it doesn't do that, then it should be renamed. somewhere.

The meaning of this docstring:

"""The following Check function takes the parameter board. Within this  """
def check(board, n):


eludes me completely. Also check suggests that we're checking something, but check is doing all sorts of things, involving taking user input. It also takes an argument n which it immediately shadows internally, and it's unclear if that's intentional or not.

WinningCon either explicitly returns True or implicitly returns None - it gives no indication as to who won. It has a variable (loop1) that isn't used.

I would strongly, strongly suggest rewriting your code with PEP-8 in mind, providing better variable names and comments, and using 4-space indentation throughout. Once you do that, you can repost and I will hopefully be able to provide more specific comments as to the actual functionality.

Code Snippets

def CreateBoard(r, c, b, n):
def print_board(b):   
    """This function takes the parameter board(b), prints
    the board and then returns the board."""
def print_board(board):
    ...
"""The following Check function takes the parameter board. Within this  """
def check(board, n):

Context

StackExchange Code Review Q#115331, answer score: 18

Revisions (0)

No revisions yet.