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

First Chess Project (ver 1.1)

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

Problem

I'd already asked for my code to be reviewed earlier here

First Chess project)

Full code

Considering the suggestions put forth for that question, I spent a day and refactored my whole code. For those who have seen my previous question/code, could you you point out anything which I have as of yet not improved upon?

For everyone else, all I want to know is whether I am on the right track while programming in python. Could you please tell me if there is something unpythonic in my code? What parts are not written in the way "good Python code" is meant to be written? Here is a snippet I feel is not really good:

  1. Taking input from user



This is in the main loop:

```
input_read=False

if len(m)==4:
X1,Y1,X2,Y2 = m # assigning final co-ordinates
m=[] # empty the buffer input list
input_read=True

if not input_read:
events=pygame.event.get()
for event in events:

if event.type == pygame.QUIT: # Quitting
exit(0)

if event.type == pygame.MOUSEBUTTONUP: pressed= False # get ready for next click
if event.type == pygame.MOUSEBUTTONDOWN and not pressed:
pressed=True
if event.button==1:
if len(m)==2: # add second click co-ordinates
pos = pygame.mouse.get_pos()
x = pos[0]/80+1
y = pos[1]/80+1
m.extend([x,y])

else:
pos = pygame.mouse.get_pos() # add first click co-ordinates
x = pos[0]/80+1
y = pos[1]/80+1
j=checkPositionToPieceDict((x,y)) # Checks that the first click has a piece
if j: # of your colour and selects it.

Solution

This code seems much better than before, so congrats. Here are a few more things to adjust:

-
Move your main game loop into a function and call it in an if block like this:

if __name__ = "__main__":
    #call your main function


This is so that people (including yourself) importing your module don't immediately get a chess window popping up.

-
You factored out the Piece baseclass from all of the pieces, simplifying that code quite a bit. This is good! But now you should finish the job of merging the different coloured pieces into the same class. They are extremely close to identical. You can pass in all of the values that distinguish them (colour, position, etc.) in the __init__ method when you initialize them.

-
At this point, with the number of global variables and global functions operating on those variables, you should group them into a GameState object to encapsulate them.

-
I will second the recommendations to replace numbers that are clearly constants with variables to improve readability, as well as follow the style guidelines in PEP 8.

Beyond that, I think the changes we're looking at are either implementation specific (you would do it differently depending on what this is supposed to accomplish, and what your goals are), are too minor to bring up one by one, or down to personal coding style.

Code Snippets

if __name__ = "__main__":
    #call your main function

Context

StackExchange Code Review Q#43262, answer score: 2

Revisions (0)

No revisions yet.