Skip to main content
Commonmark migration
Source Link

#Review

Review

#Review

Review

logic error: replace 'setter' by 'getter'
Source Link

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

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

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

Source Link

#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 setters 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.