7
\$\begingroup\$

I've written a small app using the kivy framework, which aims at reviewing your times tables. My major concern is about making this app easier to improve in the future. That is, how to organize the code to make it more clearer (by adding a User class that would store the data associated to the user, for instance). Since the code pretty straightforward, I've not added documentation to it.

The Python code:

# -*- coding: utf-8 -*-
from numpy.random import randint

import kivy
kivy.require('1.9.1-dev')
from kivy.app import App
from kivy.uix.widget import Widget
from kivy.core.window import Window
from kivy.properties import StringProperty


Window.size = (200, 100)


class MyWidget(Widget):

    help_message = StringProperty()
    question = StringProperty()
    feedback = StringProperty()

    def __init__(self):
        super(MyWidget, self).__init__()
        self.help_message = "Your answer"
        self.question = ""
        self.answer = 0
        self.points = 0
        self.attempts = 1e-10
        self.feedback = ""
        self.set_question()

    def set_question(self):
        a, b = randint(1, 11, 2)
        self.question = "{} x {} =".format(a, b)
        self.answer = a * b

    def check_answer(self, user_input):
        try:
            if int(user_input) == self.answer:
                self.points += 1
                self.feedback = "right"
            else:
                self.feedback = "{} {}".format(self.question, self.answer)
            self.set_question()
            self.attempts += 1
        except:
            self.feedback = "invalid answer"

    def set_score(self):
        score = self.points / self.attempts * 100
        self.feedback = "{:3.0f}% of right answers".format(score)


class MyApp(App):
    def build(self):
        return MyWidget()


if __name__ == '__main__':
    MyApp().run()

The .kv one:

<MyWidget>:

    canvas:
        Color:
            rgba: .93, .93, .93, 1.
        Rectangle:
            pos: self.pos
            size: self.size

    TextInput:
        size: 130, 30
        pos: 70, 30
        text: root.help_message
        background: 1., 1., 1., 1.
        font_size: 14
        multiline: False
        on_text_validate: root.check_answer(self.text); self.text = ""

    Label:
        size: 200, 30
        pos: 0, 60
        text: root.feedback
        color: .0, .0, .0, 1.
        font_size: 14

    Label:
        size: 70, 30
        pos: 0, 30
        text: root.question
        color: .0, .0, .0, 1.
        font_size: 14

    Button:
        size: 200, 30
        pos: 0, 0
        text: "Stop"
        color: .0, .0, .0, 1.
        font_size: 14
        on_release: root.set_score()

[EDIT] Mentioned code reviewer here.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Your code is very nice! There is almost nothing for me to fault.

However, your try except statement is far too large in the try and is a bare except. Let's say, for whatever reason, your .format raises an error. You could have put the correct answer, however for reason beyond your control, the program failed. And you the user gets '1 x 3 = 3', when they put 3.

To amend this you can use the else part of the try except else finally:

try:
    user_input = int(user_input)
except ValueError:
    self.feedback = "invalid answer. Please enter numbers."
else:
    if user_input == self.answer:
        self.points += 1
        self.feedback = "right"
    else:
        self.feedback = "{} {}".format(self.question, self.answer)
    self.set_question()
    self.attempts += 1

You could say that you don't want to do user_input = int(user_input). If this were the case then you can have the try as just int(user_input), and have the else as your old try.


If I were to improve this more:

  • Better class names.

    MyApp is not descriptive and is definitely not my app. TimesTabelsApp could be an alternate, unless you wish to add more to it. The same as above goes for MyWidget. You could likewise call this TimesTabelsWiget.

  • Put almost everything in the Python 'main'.

    Window.size = (200, 100), should be in the main. It will still be global!

  • Try to avoid large library's when python comes built in with things you need.

    I don't think I should need to install NumPy, when CPython already has randint. Sure, it's not as convenient, but asking an end user to install an entire library for something so small is kinda annoying. I know numpy is pip install numpy away, but Python can do this.

    from random import randint
    a, b = randint(1, 11), randint(1, 11)
    
  • A single line at the begging of each function, class and module is enough to document your code entirely.

    def set_question(self):
    """Set the internal question and answer."""
    

    Sure it's something that can be understood from the code, but what if I don't want to read the code, or can't.

  • Finally, I don't know why you need self.attempts = 1e-10. It seems kinda strange. And gives a large answer in set_score. I think it's to not get a ZeroDivisionError in set_score.

    So you could try this:

    try:
        score = self.points / self.attempts
    except ZeroDivisionError:
        self.feedback = "You didn't attempt any answers"
    else:
        self.feedback = "{:3.0f}% of right answers".format(score * 100)
    
\$\endgroup\$
1
  • \$\begingroup\$ Thank you. Can I mention you as code reviewer in the application ? \$\endgroup\$
    – he2lo
    Commented Aug 6, 2015 at 10:22

Your Answer

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

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