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()
\$\endgroup\$
  • \$\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\$ – πάντα ῥεῖ Oct 1 '18 at 22:12
  • \$\begingroup\$ I have edited it. \$\endgroup\$ – Marten Oct 1 '18 at 22:24
1
\$\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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.