3
\$\begingroup\$

I wrote a script that gives the user a quick arithmetic quiz and results using a tkinter window.

I am very new to programming, and this is also my first time using tkinter as well.

import random, tkinter as tk
from tkinter import Button, Frame, Label, Entry, Tk as initializeRoot



class TkWindows:

    def __init__(self) -> None:
        pass


    def createFrame(self, root) :
        frame = Frame(root)
        frame.pack(expand=True)
        return frame
    
    
    def modeSelection(self, windowDims: list = [300, 300], mode: str | None = None, modeOptions: list=['Addition', 'Subtraction', 'Multiplication', 'Division']) -> str :

        # Gets the chosen mode and closes the window
        def getModeChoice(selectedOption: str, root) -> None:
            nonlocal mode
            mode = selectedOption
            root.destroy()

        # Initializes the window
        root = initializeRoot()
        root.title('Select Mode')

        # Initializes window dimensions
        screenDims = [root.winfo_screenwidth(), root.winfo_screenheight()]
        windowCoords = [int((screenDims[0] - windowDims[0]) / 2), int((screenDims[1] - windowDims[1]) / 2)]
        root.geometry(f"{windowDims[0]}x{windowDims[1]}+{windowCoords[0]}+{windowCoords[1]}")

        # Adds window frame and elements
        frame = self.createFrame(root)

        for option in modeOptions:
            button = Button(frame, text=option, command=lambda opt=option: getModeChoice(opt, root))
            button.pack(pady=5, fill=tk.X)

        root.mainloop()


        return mode
    
    
    def questions(self, questionList: list, operationSymbol: str, mode: str, responseList: list = [], windowDims: list=[300,200], questionIndex: int = 0) -> list :
        
        def getResponse(root, entry) -> None:
            answers = entry.get()
            responseList.append(answers)

            nonlocal questionIndex
            questionIndex += 1

            if questionIndex < len(questionList) :
                entry.delete(0, tk.END)
                questionLabel.config(text=str(questionList[questionIndex][0]) + ' ' + operationSymbol + ' ' + str(questionList[questionIndex][1]))
                questionCounter.config(text=('Question ' + str(questionIndex + 1) + ' out of ' + str(len(questionList))))

            else :
                root.destroy()
        
        root = initializeRoot()
        root.title(str(mode))

        screenDims = [root.winfo_screenwidth(), root.winfo_screenheight()]
        windowCoords = [int((screenDims[0] - windowDims[0]) / 2), int((screenDims[1] - windowDims[1]) / 2)]

        root.geometry(f"{windowDims[0]}x{windowDims[1]}+{windowCoords[0]}+{windowCoords[1]}")

        frame = self.createFrame(root)

        questionCounter = Label(frame, text=('Question ' + str(questionIndex + 1) + ' out of ' + str(len(questionList))), font=("Arial", 16))
        questionCounter.pack(pady=5)

        questionLabel = Label(frame, text=(str(questionList[0][0]) + ' ' + operationSymbol + ' ' + str(questionList[0][1])), font=("Arial", 20))
        questionLabel.pack(pady=5)

        textBox = Entry(frame)
        textBox.pack(pady=5)

        enterButton = tk.Button(frame, text='Enter', command=lambda: getResponse(root, textBox))
        enterButton.pack(pady=5)

        tk.mainloop()
        return responseList


    def questionResults(self, questionList: list, responseList: list, operationSymbol: str, windowDims: list=[300, 250], questionsPerPage: int=5, pageIndex: int=0, correctQuestions: list=[], incorrectQuestions: list=[], incorrectQuestionsWithResponse: list=[], incorrectLabels: list=[]) -> None:
        
        for i in range(len(responseList)) :
            questionString = str(questionList[i][0]) + ' ' + operationSymbol + ' ' + str(questionList[i][1]) + ' = ' + str(questionList[i][2])
            if str(responseList[i]) == str(questionList[i][2]) :
                correctQuestions.append(questionString)
            else: 
                incorrectQuestions.append(questionString)
                incorrectQuestionsWithResponse.append(questionString + '   |   Your answer was: ' + str(responseList[i]))
            

        if len(incorrectQuestions) > 0 :

            def nextPage(root) :
                nonlocal pageIndex
                pageIndex += 1

                if pageIndex * questionsPerPage >= len(incorrectQuestions) :
                    root.destroy()
                else :
                    for i in range(questionsPerPage) :
                        try :
                            newLabel = incorrectQuestionsWithResponse[i + pageIndex * questionsPerPage]
                        except IndexError :
                            newLabel = ' '
                        incorrectLabels[i].config(text=newLabel)
            

            root = initializeRoot()
            root.title('Score: ' + str(len(correctQuestions)) + '/' + str(len(questionList)))

            screenDims = [root.winfo_screenwidth(), root.winfo_screenheight()]
            windowCoords = [int((screenDims[0] - windowDims[0]) / 2), int((screenDims[1] - windowDims[1]) / 2)]

            root.geometry(f"{windowDims[0]}x{windowDims[1]}+{windowCoords[0]}+{windowCoords[1]}")

            frame = self.createFrame(root)

            resultsTitle = Label(frame, text='You missed the following questions:')
            resultsTitle.pack(pady=5)

            for i in range(questionsPerPage) :
                try :
                    incorrectLabels.append(Label(frame, text=(incorrectQuestionsWithResponse[i])))
                except IndexError :
                    incorrectLabels.append(Label(frame, text=' '))
                incorrectLabels[i].pack(pady=5)
            
            continueButton = Button(frame, text='Continue:', command=lambda: nextPage(root))
            continueButton.pack(pady=5)

            root.mainloop()

createWindow = TkWindows()


class Main :

    def __init__(self) -> None:
        self.sessionMode: str | None = None
        self.questionList: list = []
        self.operationSymbol: str | None = None
        self.responseList: list = []
    

    def createQuestions(self, sessionMode, numOfQuestions: int=15, questionList: list=[], operationSymbol: str | None=None, operationModeDict: dict[str:str]={'Addition' : '+', 'Subtraction' : '-', 'Multiplication' : 'x', 'Division' : '÷'}) -> tuple[list, str]:
        
        operationSymbol = operationModeDict[sessionMode]

        while len(questionList) < numOfQuestions :

            questionElements = (random.randint(1,10), random.randint(1,10))

            if sessionMode == 'Addition':
                question = (questionElements[0], questionElements[1], questionElements[0] + questionElements[1])
            elif sessionMode == 'Subtraction':
                question = (questionElements[0] + questionElements[1], questionElements[0], questionElements[1])
            elif sessionMode == 'Multiplication':
                question = (questionElements[0], questionElements[1], questionElements[0] * questionElements[1])
            elif sessionMode == 'Division':
                question = (questionElements[0] * questionElements[1], questionElements[0], questionElements[1])

            if question not in questionList :
                questionList.append(question)



        return questionList, operationSymbol


    def runMain(self) :

        self.sessionMode = createWindow.modeSelection()

        if self.sessionMode != None :

            self.questionList, self.operationSymbol = self.createQuestions(self.sessionMode)

            self.responseList = createWindow.questions(self.questionList, self.operationSymbol, self.sessionMode)

            createWindow.questionResults(self.questionList, self.responseList, self.operationSymbol)

main = Main()


main.runMain()
\$\endgroup\$

3 Answers 3

Reset to default
2
\$\begingroup\$

TkWindows is not a useful class; it has no member variables. Either convert it to a module, or flatten it to a set of functions in the global namespace.

You're missing some typehints, such as root: tk.Tk, and questionList: list which needs an inner type.

You already have import tkinter as tk; you should keep it and remove your from tkinter import so that all of the library symbols can stay namespace-qualified.

It doesn't help to represent the window dimensions as a tuple - just represent them as individual variables. Write a utility method to apply your geometry string instead of copying and pasting that code.

You haven't made a very consistent decision between representing state in classes and representing state in closure variables (nonlocal). I show a more consistent application of closures, plus some immutable classes to represent questions and answers.

Don't int(x/2); use floor division x // 2.

getResponse() should not accept a root or entry parameter; you can closure-bind to root and textBox in the outer scope.

You should make better use of StringVar and IntVar; tk offers these so that you can talk about the data more than you talk about the user interface.

Don't use the Latin letter x for multiplication. You've correctly used the Unicode character for division; do the same for multiplication.

Avoid calling into random.randint directly; this will make unit testing diffcult. Instead, accept a random generator object. If and when you add unit tests, you can pass it a random generator that has had a constant seed set.

Don't != None; use is not None instead since None is a singleton.

Use the operator module to give you your simple binary function references rather than needing to write out the expressions yourself.

Lots of other things to improve, but this is a start:

Closure style

import functools
import itertools
import operator
import random
import tkinter as tk
from typing import Callable, Iterator, Sequence

from mypy.build import NamedTuple


class Operation(NamedTuple):
    name: str
    symbol: str
    fun: Callable[[int, int], int]


MODES = (
    Operation(name='Addition', symbol='+', fun=operator.add),
    Operation(name='Subtraction', symbol='-', fun=operator.sub),
    Operation(name='Multiplication', symbol='×', fun=operator.mul),
    Operation(name='Division', symbol='÷', fun=operator.truediv),
)


class Question(NamedTuple):
    operation: Operation
    a: int
    b: int

    @classmethod
    def random(cls, operation: Operation, rand: random.Random) -> 'Question':
        return cls(
            operation=operation,
            a=rand.randint(1, 10),
            b=rand.randint(1, 10),
        )

    def __str__(self) -> str:
        return f'{self.a} {self.operation.symbol} {self.b}'

    @property
    def answer(self) -> int:
        return self.operation.fun(self.a, self.b)


class Answer(NamedTuple):
    question: Question
    answer: int

    @property
    def is_correct(self) -> bool:
        return self.answer == self.question.answer

    def __str__(self) -> str:
        return (
            f'{self.question} = {self.question.answer}'
            f'   |   Your answer was: {self.answer}'
        )


def create_frame(root: tk.Tk) -> tk.Frame:
    frame = tk.Frame(root)
    frame.pack(expand=True)
    root.after(ms=1, func=root.focus_force)
    return frame


def set_geometry(
    root: tk.Tk, width: int, height: int,
) -> None:
    left = (root.winfo_screenwidth() - width)//2
    top = (root.winfo_screenheight() - height)//2
    root.geometry(f'{width}x{height}+{left}+{top}')


def mode_selection(
    width: int = 300, height: int = 300,
) -> Operation | None:
    def select(selected_option: Operation) -> None:
        nonlocal option
        option = selected_option
        root.destroy()

    root = tk.Tk()
    root.title('Select Mode')
    set_geometry(root=root, width=width, height=height)
    frame = create_frame(root)

    for mode in MODES:
        button = tk.Button(
            frame, text=mode.name,
            command=functools.partial(select, mode),
        )
        button.pack(pady=5, fill=tk.X)

    option: Operation | None = None
    root.mainloop()
    return option


def ask_questions(
    questions: Sequence[Question],
    mode: Operation,
    width: int = 300,
    height: int = 200,
) -> list[int]:
    responses: list[int] = []
    question_iter = enumerate(questions, start=1)

    def advance() -> None:
        try:
            question_index, question = next(question_iter)
        except StopIteration:
            root.destroy()
            return

        question_label.config(text=str(question))
        question_counter.config(
            text=f'Question {question_index} out of {len(questions)}',
        )
        answer_var.set(value=0)
        entry.focus_set()
        entry.select_range(0, 'end')

    def get_response(*args) -> None:
        responses.append(answer_var.get())
        advance()

    root = tk.Tk()
    root.title(mode.name)
    root.bind('<Return>', get_response)
    set_geometry(root=root, width=width, height=height)
    frame = create_frame(root)

    question_counter = tk.Label(frame, font=('Arial', 16))
    question_counter.pack(pady=5)
    question_label = tk.Label(frame, font=('Arial', 20))
    question_label.pack(pady=5)
    answer_var = tk.IntVar(frame, name='answer')
    entry = tk.Entry(frame, textvariable=answer_var)
    entry.pack(pady=5)
    tk.Button(frame, text='Enter', command=get_response).pack(pady=5)

    advance()
    root.mainloop()
    return responses


def show_results(
    questions: Sequence[Question],
    responses: Sequence[int],
    width: int = 300,
    height: int = 250,
    questions_per_page: int = 5,
) -> None:
    answers = [
        Answer(question=question, answer=answer)
        for question, answer in zip(questions, responses)
    ]
    n_correct = sum(1 for answer in answers if answer.is_correct)
    all_correct = n_correct == len(questions)
    paged_answers = itertools.batched(
        (answer for answer in answers if not answer.is_correct),
        n=questions_per_page,
    )

    def next_page(*args) -> None:
        try:
            batch = next(paged_answers)
        except StopIteration:
            root.destroy()
            return

        for label, answer in zip(incorrect_labels, batch):
            label.config(text=str(answer))
        for label in incorrect_labels[len(batch):]:
            label.config(text='')

    root = tk.Tk()
    root.title(f'Score: {n_correct}/{len(questions)}')
    root.bind('<Return>', next_page)
    set_geometry(root=root, width=width, height=height)
    frame = create_frame(root)

    results_title = tk.Label(
        frame,
        text=f'You missed {"no" if all_correct else "the following"} questions',
    )
    results_title.pack(pady=5)

    incorrect_labels = [
        tk.Label(frame)
        for _ in range(questions_per_page)
    ]
    for label in incorrect_labels:
        label.pack(pady=5)

    tk.Button(frame, text='Continue', command=next_page).pack(pady=5)

    if not all_correct:
        next_page()
    root.mainloop()


def create_questions(
    operation: Operation,
    num_of_questions: int = 15,
    rand: random.Random | None = None,
) -> Iterator[Question]:
    if rand is None:
        rand = random.Random()
    for _ in range(num_of_questions):
        yield Question.random(
            operation=operation, rand=rand,
        )


def run_main() -> None:
    session_mode = mode_selection()
    if session_mode is None:
        return

    questions = tuple(create_questions(session_mode))
    responses = ask_questions(questions, session_mode)
    show_results(questions=questions, responses=responses)


if __name__ == '__main__':
    run_main()

OOP style

import functools
import itertools
import operator
import random
import tkinter as tk
from typing import Callable, Iterator, Sequence

from mypy.build import NamedTuple


class Operation(NamedTuple):
    name: str
    symbol: str
    fun: Callable[[int, int], int]


MODES = (
    Operation(name='Addition', symbol='+', fun=operator.add),
    Operation(name='Subtraction', symbol='-', fun=operator.sub),
    Operation(name='Multiplication', symbol='×', fun=operator.mul),
    Operation(name='Division', symbol='÷', fun=operator.truediv),
)


class Question(NamedTuple):
    operation: Operation
    a: int
    b: int

    @classmethod
    def random(cls, operation: Operation, rand: random.Random) -> 'Question':
        return cls(
            operation=operation,
            a=rand.randint(1, 10),
            b=rand.randint(1, 10),
        )

    def __str__(self) -> str:
        return f'{self.a} {self.operation.symbol} {self.b}'

    @property
    def answer(self) -> int:
        return self.operation.fun(self.a, self.b)


class Answer(NamedTuple):
    question: Question
    answer: int

    @property
    def is_correct(self) -> bool:
        return self.answer == self.question.answer

    def __str__(self) -> str:
        return (
            f'{self.question} = {self.question.answer}'
            f'   |   Your answer was: {self.answer}'
        )


def create_frame(root: tk.Tk) -> tk.Frame:
    frame = tk.Frame(root)
    frame.pack(expand=True)
    root.after(ms=1, func=root.focus_force)
    return frame


def set_geometry(
    root: tk.Tk, width: int, height: int,
) -> None:
    left = (root.winfo_screenwidth() - width)//2
    top = (root.winfo_screenheight() - height)//2
    root.geometry(f'{width}x{height}+{left}+{top}')


class ModeSelection:
    def __init__(
        self, width: int = 300, height: int = 300,
    ) -> None:
        self.root = tk.Tk()
        self.root.title('Select Mode')
        self.mainloop = self.root.mainloop
        set_geometry(root=self.root, width=width, height=height)
        frame = create_frame(self.root)

        for mode in MODES:
            button = tk.Button(
                frame, text=mode.name,
                command=functools.partial(self.select, mode),
            )
            button.pack(pady=5, fill=tk.X)

        self.mode: Operation | None = None

    def select(self, selected_option: Operation) -> None:
        self.mode = selected_option
        self.root.destroy()


class QuestionPrompt:
    def __init__(
        self,
        questions: Sequence[Question],
        width: int = 300, height: int = 200,
    ) -> None:
        self.root = tk.Tk()
        self.root.title(questions[0].operation.name)
        self.root.bind('<Return>', self.get_response)
        self.mainloop = self.root.mainloop
        set_geometry(root=self.root, width=width, height=height)
        frame = create_frame(self.root)

        self.counter_var = tk.StringVar(name='counter')
        question_counter = tk.Label(frame, textvariable=self.counter_var, font=('Arial', 16))
        question_counter.pack(pady=5)

        self.question_var = tk.StringVar(name='question')
        question_label = tk.Label(frame, textvariable=self.question_var, font=('Arial', 20))
        question_label.pack(pady=5)

        self.answer_var = tk.IntVar(frame, name='answer')
        self.entry = tk.Entry(frame, textvariable=self.answer_var)
        self.entry.pack(pady=5)

        tk.Button(frame, text='Enter', command=self.get_response).pack(pady=5)

        self.questions = questions
        self.responses: list[int] = []
        self.question_iter = enumerate(questions, start=1)

        self.advance()

    def advance(self) -> None:
        try:
            question_index, question = next(self.question_iter)
        except StopIteration:
            self.root.destroy()
            return

        self.question_var.set(str(question))
        self.counter_var.set(
            f'Question {question_index} out of {len(self.questions)}',
        )
        self.answer_var.set(value=0)
        self.entry.focus_set()
        self.entry.select_range(0, 'end')

    def get_response(self, *args) -> None:
        self.responses.append(self.answer_var.get())
        self.advance()


class Results:
    def __init__(
        self,
        questions: Sequence[Question],
        responses: Sequence[int],
        width: int = 300, height: int = 250,
        questions_per_page: int = 5,
    ) -> None:
        answers = [
            Answer(question=question, answer=answer)
            for question, answer in zip(questions, responses)
        ]
        n_correct = sum(1 for answer in answers if answer.is_correct)
        all_correct = n_correct == len(questions)
        self.paged_answers = itertools.batched(
            (answer for answer in answers if not answer.is_correct),
            n=questions_per_page,
        )

        self.root = tk.Tk()
        self.root.title(f'Score: {n_correct}/{len(questions)}')
        self.root.bind('<Return>', self.next_page)
        self.mainloop = self.root.mainloop
        set_geometry(root=self.root, width=width, height=height)
        frame = create_frame(self.root)

        results_title = tk.Label(
            frame,
            text=f'You missed {"no" if all_correct else "the following"} questions',
        )
        results_title.pack(pady=5)

        self.incorrect_vars = [
            tk.StringVar(name=f'incorrect_{i}')
            for i in range(questions_per_page)
        ]

        for var in self.incorrect_vars:
            tk.Label(frame, textvariable=var).pack(pady=5)

        tk.Button(frame, text='Continue', command=self.next_page).pack(pady=5)

        if not all_correct:
            self.next_page()

    def next_page(self, *args) -> None:
        try:
            batch = next(self.paged_answers)
        except StopIteration:
            self.root.destroy()
            return

        for label, answer in zip(self.incorrect_vars, batch):
            label.set(str(answer))
        for label in self.incorrect_vars[len(batch):]:
            label.set('')


def create_questions(
    operation: Operation,
    num_of_questions: int = 15,
    rand: random.Random | None = None,
) -> Iterator[Question]:
    if rand is None:
        rand = random.Random()
    for _ in range(num_of_questions):
        yield Question.random(
            operation=operation, rand=rand,
        )


def run_main() -> None:
    selection = ModeSelection()
    selection.mainloop()
    if selection.mode is None:
        return

    questions = tuple(create_questions(selection.mode))

    prompt = QuestionPrompt(questions=questions)
    prompt.mainloop()

    results = Results(questions=questions, responses=prompt.responses)
    results.mainloop()


if __name__ == '__main__':
    run_main()
\$\endgroup\$
1
\$\begingroup\$

Features

It is good that the user has a choice of operations (add, subtract, etc.). It would also be nice for the user to select how many questions to answer, instead of always having to answer 15.

It is good that the user sees a summary of incorrect answers, but it would also be nice to see a confirmation message that all questions were answered correctly.

It is nice that the code gracefully handles unexpected input, such as words instead of numbers.

Long lines

Very long lines are hard to understand. You can use the black program to automatically modify the code for you for better readability.

Layout

The use of vertical whitespace is inconsistent. The black program can help with that, too.

Documentation

Add docstrings for your classes and functions to help describe the purpose of the code.

For example, in the function modeSelection, what is a mode?

In the function questions, what is the function doing with questions?

Naming

Refer to the PEP 8 style guide for recommendations on how to name functions and variables.

Simpler

This line:

    if len(incorrectQuestions) > 0 :

is simpler as:

    if len(incorrectQuestions):
\$\endgroup\$
1
  • \$\begingroup\$ if len(incorrectQuestions): is probably the worst of both worlds. The pythonic way is if incorrect_questions and only that, standard library collections are implicitly false iff they are empty. Notable exception (damn, and it hurts) is np.array, where if len(my_array) is reasonable, but there is no numpy here. \$\endgroup\$ Commented Apr 20, 2024 at 12:46
1
\$\begingroup\$

types

annotation

    def __init__(self) -> None:
        pass


    def createFrame(self, root) :

Thank you for the -> None annotation -- it gives me some measure of confidence that I could usefully run mypy against this codebase. But then, oddly, the next method isn't annotated.

Consider giving mypy the --strict switch, and follow its advice.

PEP-8 asked you nicely to call your methods create_frame(), mode_selection(), and so on. If a library such as tkinter requires that you use camelCase, or if your development organization's style guide uses it, then note that in a comment or ReadMe.

Also, please improve the legibility of this code by running "$ black -S *.py", so we're not ending each signature with a distracting SPACE colon. And use "$ isort *.py" to make your imports match PEP-8.

Please delete the __init__ constructor altogether, as it does nothing beyond what the cPython interpreter would already offer by default.

tuple vs list

    def modeSelection(self, windowDims: list = [300, 300], ...

I hope your type linter is telling you that window_dims: list[int] would be a better annotation. But here, there's more than one reason you should instead adopt tuple[int, int].

  1. In python you should prefer tuple for a C-struct where position matters, such as for a (width, height) or (x, y) pair. Use a list for an arbitrary number of "same thing".
  2. Avoid mutable default arguments, as they can lead to surprising debug trouble.

Had you run "$ pylint *.py" it would have offered this advice:

W0102: Dangerous default value [] as argument (dangerous-default-value)

nested function

Grrrrrrr...

    def questions(self, ... , questionIndex: int = 0) -> list :
        
        def getResponse(root, entry) -> None:
            ...
            nonlocal questionIndex
            questionIndex += 1
  1. Avoid nesting a function in that way -- put it at class level so your test suite can access it and so you avoid coupling issues.
  2. If there's some convincing reason why nesting is the best approach for a given situation, still avoid the nonlocal keyword, preferring to pass params differently, perhaps by manipulating self.question_index.

extract helper

Consider moving common code from question() and questionResults() into a suitably parameterized helper method.

main guard

main = Main()
main.runMain()

These module-level statements produce side effects, and prevent your automated test suite from safely importing this module. Surround them with the usual guard:

if __name__ == "__main__":
    Main().run_main()
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.