2
\$\begingroup\$

I have made a little Snake game with Python tkinter. It's my first Python program with one GUI. What can I improve?

from tkinter import *
import random
boardWidth = 30
boardHeight = 30
tilesize = 10
class Snake():
 def __init__(self):
 self.snakeX = [20, 20, 20]
 self.snakeY = [20, 21, 22]
 self.snakeLength = 3
 self.key = "w"
 self.points = 0
 def move(self): # move and change direction with wasd
 for i in range(self.snakeLength - 1, 0, -1):
 self.snakeX[i] = self.snakeX[i-1]
 self.snakeY[i] = self.snakeY[i-1]
 if self.key == "w":
 self.snakeY[0] = self.snakeY[0] - 1
 elif self.key == "s":
 self.snakeY[0] = self.snakeY[0] + 1
 elif self.key == "a":
 self.snakeX[0] = self.snakeX[0] - 1
 elif self.key == "d":
 self.snakeX[0] = self.snakeX[0] + 1
 self.eatApple()
 def eatApple(self):
 if self.snakeX[0] == apple.getAppleX() and self.snakeY[0] == apple.getAppleY():
 self.snakeLength = self.snakeLength + 1
 x = self.snakeX[len(self.snakeX)-1] # Snake grows
 y = self.snakeY[len(self.snakeY) - 1]
 self.snakeX.append(x+1)
 self.snakeY.append(y)
 self.points = self.points + 1
 apple.createNewApple()
 def checkGameOver(self):
 for i in range(1, self.snakeLength, 1):
 if self.snakeY[0] == self.snakeY[i] and self.snakeX[0] == self.snakeX[i]:
 return True # Snake eat itself
 if self.snakeX[0] < 1 or self.snakeX[0] >= boardWidth-1 or self.snakeY[0] < 1 or self.snakeY[0] >= boardHeight-1:
 return True # Snake out of Bounds
 return False
 def getKey(self, event):
 if event.char == "w" or event.char == "d" or event.char == "s" or event.char == "a" or event.char == " ":
 self.key = event.char
 def getSnakeX(self, index):
 return self.snakeX[index]
 def getSnakeY(self, index):
 return self.snakeY[index]
 def getSnakeLength(self):
 return self.snakeLength
 def getPoints(self):
 return self.points
class Apple:
 def __init__(self):
 self.appleX = random.randint(1, boardWidth - 2)
 self.appleY = random.randint(1, boardHeight - 2)
 def getAppleX(self):
 return self.appleX
 def getAppleY(self):
 return self.appleY
 def createNewApple(self):
 self.appleX = random.randint(1, boardWidth - 2)
 self.appleY = random.randint(1, boardHeight - 2)
class GameLoop:
 def repaint(self):
 canvas.after(200, self.repaint)
 canvas.delete(ALL)
 if snake.checkGameOver() == False:
 snake.move()
 snake.checkGameOver()
 canvas.create_rectangle(snake.getSnakeX(0) * tilesize, snake.getSnakeY(0) * tilesize,
 snake.getSnakeX(0) * tilesize + tilesize,
 snake.getSnakeY(0) * tilesize + tilesize, fill="red") # Head
 for i in range(1, snake.getSnakeLength(), 1):
 canvas.create_rectangle(snake.getSnakeX(i) * tilesize, snake.getSnakeY(i) * tilesize,
 snake.getSnakeX(i) * tilesize + tilesize,
 snake.getSnakeY(i) * tilesize + tilesize, fill="blue") # Body
 canvas.create_rectangle(apple.getAppleX() * tilesize, apple.getAppleY() * tilesize,
 apple.getAppleX() * tilesize + tilesize,
 apple.getAppleY() * tilesize + tilesize, fill="green") # Apple
 else: # GameOver Message
 canvas.delete(ALL)
 canvas.create_text(150, 100, fill="darkblue", font="Times 20 italic bold", text="GameOver!")
 canvas.create_text(150, 150, fill="darkblue", font="Times 20 italic bold",
 text="Points:" + str(snake.getPoints()))
snake = Snake()
apple = Apple()
root = Tk()
canvas = Canvas(root, width=300, height=300)
canvas.configure(background="yellow")
canvas.pack()
gameLoop = GameLoop()
gameLoop.repaint()
root.title("Snake")
root.bind('<KeyPress>', snake.getKey)
root.mainloop()
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 1, 2018 at 22:08
\$\endgroup\$
3
  • 1
    \$\begingroup\$ "In my posted code the indentations are partly wrong, because I couldn't format it correctly." Indentations are essential for correct python code. Read up here how to post code. \$\endgroup\$ Commented Oct 1, 2018 at 22:12
  • \$\begingroup\$ I have edited it. \$\endgroup\$ Commented Oct 1, 2018 at 22:24
  • \$\begingroup\$ How would you repeat the game? Ie, to create a restart button without re-running the code? \$\endgroup\$ Commented May 23, 2022 at 0:14

1 Answer 1

2
\$\begingroup\$

Review

Naming conventions

In Python, methods and variables should be named the following way: https://stackoverflow.com/questions/159720/what-is-the-naming-convention-in-python-for-variable-and-function-names So in your case, it would be:

  • self.snakeLength > self.snake_length
  • def eatApple(self) > def eat_apple(self)

Choose better names for variables

Snake.snakeX seems to be a little bit too much. Why don't you

  • Snake.snakeX > Snake.x
  • Snake.snakeLength > Snake.length
  • Apple.appleX > Apple.x

Use class constants instead of global variables

Usually you want to have as few golbal variables / constants as possible. Instead of boardWidth = 30 you can create a cosntant and move it to some class, like BOARD_WIDTH = 30 (see example).

Wrap the main code

Create a new class which wraps the code your script runs. You could, for exapmple, use the Tk class (see example)

Use main

If someone wants to import your code, he will instatnly run the whole script, creating a Tkinter widget, running the gameloop infinitely. Instead, wrap it with a if __name__ == "__main__": statement.

Prevent the snake of moving backwards

In your implementation it is possible to do a 180° turn, which kills your snake instantly. You could always save the last move, e.g. "w", which blocks all incoming "s", so only "w", "a" and "d" are valid for the next move (see example).

Reduce visibility

You can reduce the visibility of class members by placing __ in front of them. For example

  • self.apple > self.__apple

Looks ugly, but this way these fields can't be accessed from outside your class.

Use @property

Instead of defining getters manually, use propteries (see example). More information: https://www.smallsurething.com/private-methods-and-attributes-in-python/

Code improvements

Instead of event.char == "w" or event.char == "d" or event.char == "s" or event.char == "a" you can define a list of all keys KEYS = ["w", "a", "s", "d"] and then do event.char in Snake.KEYS

Do not write if snake.checkGameOver() == False, instead do if not snake.checkGameOver()

You might use type hinting

Since python 3.5 there is typehinting (see example). More information: https://blog.jetbrains.com/pycharm/2015/11/python-3-5-type-hinting-in-pycharm-5/

Code example

snake.py

from tkinter import *
import random
from typing import List
class Apple:
 def __init__(self):
 self.__x = random.randint(1, App.BOARD_WIDTH - 2)
 self.__y = random.randint(1, App.BOARD_HEIGHT - 2)
 def create_new_apple(self) -> None:
 self.__x = random.randint(1, App.BOARD_WIDTH - 2)
 self.__y = random.randint(1, App.BOARD_HEIGHT - 2)
 @property
 def x(self) -> int:
 return self.__x
 @property
 def y(self) -> int:
 return self.__y
 
class Snake:
 KEYS = ["w", "a", "s", "d"]
 MAP_KEY_OPP = {"w": "s", "a": "d", "s": "w", "d": "a"}
 def __init__(self, apple):
 self.__apple = apple
 self.__x = [20, 20, 20]
 self.__y = [20, 21, 22]
 self.__length = 3
 self.__key_current = "w"
 self.__key_last = self.__key_current
 self.__points = 0
 def move(self) -> None: # move and change direction with wasd
 self.__key_last = self.__key_current
 for i in range(self.length - 1, 0, -1):
 self.__x[i] = self.__x[i - 1]
 self.__y[i] = self.__y[i - 1]
 if self.__key_current == "w":
 self.__y[0] = self.__y[0] - 1
 elif self.__key_current == "s":
 self.__y[0] = self.__y[0] + 1
 elif self.__key_current == "a":
 self.__x[0] = self.__x[0] - 1
 elif self.__key_current == "d":
 self.__x[0] = self.__x[0] + 1
 self.eat_apple()
 def eat_apple(self) -> None:
 if self.__x[0] == self.__apple.x and self.__y[0] == self.__apple.y:
 self.__length = self.__length + 1
 x = self.__x[len(self.__x) - 1] # snake grows
 y = self.__y[len(self.__y) - 1]
 self.__x.append(x + 1)
 self.__y.append(y)
 self.__points = self.__points + 1
 self.__apple.create_new_apple()
 @property
 def gameover(self) -> bool:
 for i in range(1, self.length, 1):
 if self.__y[0] == self.__y[i] and self.__x[0] == self.__x[i]:
 return True # snake ate itself
 if self.__x[0] < 1 or self.__x[0] >= App.BOARD_WIDTH - 1 or self.__y[0] < 1 or self.__y[0] >= App.BOARD_HEIGHT - 1:
 return True # snake out of bounds
 return False
 def set_key_event(self, event: Event) -> None:
 if event.char in Snake.KEYS and event.char != Snake.MAP_KEY_OPP[self.__key_last]:
 self.__key_current = event.char
 @property
 def x(self) -> List[int]:
 return self.__x.copy()
 @property
 def y(self) -> List[int]:
 return self.__y.copy()
 @property
 def length(self) -> int:
 return self.__length
 @property
 def points(self) -> int:
 return self.__points
class App(Tk):
 BOARD_WIDTH = 30
 BOARD_HEIGHT = 30
 TILE_SIZE = 10
 COLOR_BACKGROUND = "yellow"
 COLOR_SNAKE_HEAD = "red"
 COLOR_SNAKE_BODY = "blue"
 COLOR_APPLE = "green"
 COLOR_FONT = "darkblue"
 FONT = "Times 20 italic bold"
 FONT_DISTANCE = 25
 TEXT_TITLE = "Snake"
 TEXT_GAMEOVER = "GameOver!"
 TEXT_POINTS = "Points: "
 TICK_RATE = 200 # in ms
 
 def __init__(self, screenName=None, baseName=None, className='Tk', useTk=1, sync=0, use=None):
 Tk.__init__(self, screenName, baseName, className, useTk, sync, use)
 
 self.__apple = Apple()
 self.__snake = Snake(self.__apple)
 self.__canvas = Canvas(self, width=App.BOARD_WIDTH * App.TILE_SIZE, height=App.BOARD_HEIGHT * App.TILE_SIZE)
 self.__canvas.pack()
 self.__canvas.configure(background=App.COLOR_BACKGROUND)
 
 self.title(App.TEXT_TITLE)
 self.bind('<KeyPress>', self.__snake.set_key_event)
 def mainloop(self, n=0):
 self.__gameloop()
 Tk.mainloop(self, n)
 def __gameloop(self):
 self.after(App.TICK_RATE, self.__gameloop)
 self.__canvas.delete(ALL)
 if not self.__snake.gameover:
 self.__snake.move()
 x = self.__snake.x
 y = self.__snake.y
 self.__canvas.create_rectangle(
 x[0] * App.TILE_SIZE,
 y[0] * App.TILE_SIZE,
 x[0] * App.TILE_SIZE + App.TILE_SIZE,
 y[0] * App.TILE_SIZE + App.TILE_SIZE,
 fill=App.COLOR_SNAKE_HEAD
 ) # Head
 for i in range(1, self.__snake.length, 1):
 self.__canvas.create_rectangle(
 x[i] * App.TILE_SIZE,
 y[i] * App.TILE_SIZE,
 x[i] * App.TILE_SIZE + App.TILE_SIZE,
 y[i] * App.TILE_SIZE + App.TILE_SIZE,
 fill=App.COLOR_SNAKE_BODY
 ) # Body
 self.__canvas.create_rectangle(
 self.__apple.x * App.TILE_SIZE,
 self.__apple.y * App.TILE_SIZE,
 self.__apple.x * App.TILE_SIZE + App.TILE_SIZE,
 self.__apple.y * App.TILE_SIZE + App.TILE_SIZE,
 fill=App.COLOR_APPLE
 ) # Apple
 else: # GameOver Message
 x = App.BOARD_WIDTH * App.TILE_SIZE / 2 # x coordinate of screen center
 y = App.BOARD_HEIGHT * App.TILE_SIZE / 2 # y coordinate of screen center
 self.__canvas.create_text(x, y - App.FONT_DISTANCE, fill=App.COLOR_FONT, font=App.FONT,
 text=App.TEXT_GAMEOVER)
 self.__canvas.create_text(x, y + App.FONT_DISTANCE, fill=App.COLOR_FONT, font=App.FONT,
 text=App.TEXT_POINTS + str(self.__snake.points))
 
if __name__ == "__main__":
 App().mainloop()

As always, no warranty. As there are many different styles for programming python, these are some suggestions for you, how you COULD impove your code. Many things I mentioned are optional, not necessary.

answered Oct 8, 2018 at 0:58
\$\endgroup\$
0

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.