2
\$\begingroup\$

A while back I had to do a project for school in Python, creating a game or something interesting. I decided to make a memory game. The code isn't really nice and neither are the variable names. It's not supposed to look good, and all that matters is the functionality. I had done a lot of movements with the Turtle so going through all of it again and shortening the moves wasn't really my goal. There's too much code and I had no more nerves. All I'm asking for is your overall opinion of how it was done and how it works.

from turtle import *
import time
from random import *
from tkinter import *

speed(0)
def mj(x, y):
    return x, y

def crtanje2(x, y):
    global pon
    global m
    global klik
    global tocnih
    pon += 1
    klik += 1
    for i in range(20):
        if x >= unutar[i][0] and x <= unutar[i][0]+100 and y <= unutar[i][1]+100 and y >= unutar[i][1]:
            pu()
            goto(unutar[i][0]+50, unutar[i][1]+25)
            pd()
            rj = l.index(i)
            rj2 = l.index(i)
            if rj % 2 != 0:
                rj -= 1
            crtanje(rj, boja[rj//2])
            m[1-pon%2] = rj2
            break
    if pon % 2 == 0 and pon != 0 and m[0] != -101 and m[1] != -100 and abs(m[0]-m[1]) != 1 or abs(m[0]-m[1]) == 1 and min(m[0], m[1]) % 2 != 0:
        tracer(True)
        time.sleep(1)
        tracer(False)
        pu()
        goto(unutar[l[m[0]]][0]+1, unutar[l[m[0]]][1]+1)
        pd()
        pencolor('white')
        fillcolor('white')
        begin_fill()
        for i in range(4):
             fd(98)
             lt(90)
        end_fill()
        pu()
        goto(unutar[l[m[1]]][0]+1, unutar[l[m[1]]][1]+1)
        pd()
        pencolor('white')
        fillcolor('white')
        begin_fill()
        for i in range(4):
            fd(98)
            lt(90)
        end_fill()
    if abs(m[0]-m[1]) == 1 and min(m[0], m[1]) % 2 == 0:
        tocnih += 1
    if pon % 2 == 0:
        m = [-101, -100]
    pencolor('black')
    if tocnih == 10:
        import sys; sys.exit('\n----------------------\nSolved in {} steps!\n----------------------'.format(pon//2))
    return


def tablica():
    hideturtle()
    pu()
    goto(-250, 100)
    pd()
    for i in range(20):
        if i % 5 == 0 and i != 0:
            bk(500)
            rt(90)
            fd(100)
            lt(90)
        kvadrat()
        fd(100)

def kvadrat():
    for i in range(4):
        fd(100)
        lt(90)

def pravokutnik():
    pu()
    bk(25)
    lt(90)
    fd(12.5)
    rt(90)
    pd()
    begin_fill()
    for i in range(4):
        if i % 2 == 0:
            fd(50)
        else:
            fd(25)
        lt(90)

def zvijezda():
    pu()
    lt(90)
    fd(37.5)
    lt(90)
    fd(25)
    rt(180)
    pd()
    begin_fill()
    for i in range(5):
        fd(50)
        rt(144)

def paralelogram():
    pu()
    lt(90)
    fd(12.5)
    rt(90)
    pd()
    for i in range(4):
        if i % 2 == 0:
            fd(25)
            lt(140)
        else:
            fd(25)
            lt(40)

def trapez():
    pu()
    lt(90)
    fd(12.5)
    rt(90)
    bk(17.5)
    pd()
    begin_fill()
    fd(50)
    lt(140)
    fd(25)
    lt(40)
    fd(25)
    lt(40)
    fd(25)
    lt(140)
    fd(25)

def polukrug():
    pu()
    fd(25)
    lt(90)
    fd(12.5)
    pd()
    begin_fill()
    circle(25, extent = 180)
    lt(90)
    fd(50)

def crtanje(br, boja):
    fillcolor(boja)
    if br == 0:
        begin_fill()
        circle(25, steps = 4)
    elif br == 2:
        begin_fill()
        circle(25)
    elif br == 4:
        begin_fill()
        circle(25, steps = 3)
    elif br == 6:
        pravokutnik()
    elif br == 8:
        pu()
        bk(20)
        lt(90)
        fd(12.5)
        rt(90)
        pd()
        rt(45)
        begin_fill()
        circle(25, steps = 4)
        lt(45)
    elif br == 10:
        pu()
        bk(20)
        lt(90)
        fd(20)
        rt(90)
        pd()
        begin_fill()
        for i in range(5):
            circle(5)
            pu()
            fd(10)
            pd()
    elif br == 12:
        zvijezda()
    elif br == 14:
        begin_fill()
        paralelogram()
    elif br == 16:
        trapez()
    else:
        polukrug()
    end_fill()

tracer(False)
l = sample(range(0, 20), 20)
tablica()
colormode(255)
boja = []
for i in range(0, 20, 2):
    boja.append((randint(0, 255), randint(0, 255), randint(0, 255)))
    for j in range(i, i+2):
        pu()
        goto(-200+((l[j]%5)*100), 125-((l[j]//5)*100))
        pd()
        crtanje(i, boja[i//2])
tracer(True)
time.sleep(10)
reset()
tracer(False)
tablica()
unutar = [(-250, 100), (-150, 100), (-50, 100), (50, 100), (150, 100), (-250, 0), (-150, 0), (-50, 0), (50, 0), (150, 0), (-250, -100), (-150, -100), (-50, -100), (50, -100), (150, -100), (-250, -200), (-150, -200), (-50, -200), (50, -200), (150, -200)]
tocno = 0
tracer(True)
tracer(False)
pon = 0
tocnih = 0
m = [-101, -100]
klik = 0
poz = onscreenclick(crtanje2)
mainloop()
\$\endgroup\$
  • \$\begingroup\$ Your code is very very long. Try running pylint on your code and after fixing everything mentioned, may ask more precise questions on a simplified (maybe schematic) code. \$\endgroup\$ – sebix Nov 28 '14 at 19:54
3
\$\begingroup\$

All I'm asking for is your overall opinion of how it was done and how it works.

Not good. Your code breaks almost any rule generally agreed about good code. But don't be sad, you are lucky because you can now learn some good principles and write better code in the future.


from turtle import *
import time
from random import *
from tkinter import *

Please do not use import *, instead use import long_long_name as short


crtanje2(x, y) and crtanje(br, boja) are 50 lines long. Divide them in smaller functions.


Remove the following because it is never used.

def mj(x, y):
    return x, y

global pon
global m
global klik
global tocnih

Do not use globals, instead pass the variables you need to the function as parametre.


Here code written in the English language is the most welcome, yours isn't, please consider translating your function names to English.


pu()
lt(90)
fd(37.5)
lt(90)
fd(25)
rt(180)
pd()

Your code uses a great deal of two letters names, longer names are generally preferred.


It is common practice to define a main function that actually does stuff and then do:

if __name__ == "__main__":
    main()

so that you can then import your module.


unutar = [(-250, 100), (-150, 100), (-50, 100), (50, 100), (150, 100), (-250, 0), (-150, 0), (-50, 0), (50, 0), (150, 0), (-250, -100), (-150, -100), (-50, -100), (50, -100), (150, -100), (-250, -200), (-150, -200), (-50, -200), (50, -200), (150, -200)]

This is a long list that maybe could be generated with list comprehension.


You are giving the programme all the info to draw the images by itself, this is quite time consuming and very hard to change.

Use the right tool for the right job

Pygame supports sprite (a fancy term for image) loading so you can have a resources folder with all the images of the objects that you want to draw and you can change them anytime.


When you have fixed all the more serious issues fix what PEP8 tells you to fix.

\$\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.