6
\$\begingroup\$

I thought that developing an app in Python should be easier than learning Java. I finally got my isprime app to work but it looks ugly. When I write GUI code I always find myself typing in very long code. This is not Code Golf for sure, but 70 lines for such a simple app is too much in opinion. The shorter (reasonably) a programme is, the easier it is to debug.

This app gives the user a text input and a button, and clicking the button will tell the user whether or not the number entered is prime.

That said, if you own an Android device you can download the app (.apk) here. Please tell me about any improvements.

import time
import random
from kivy.app import App
from kivy.uix.button import Label
from kivy.uix.textinput import TextInput
from kivy.uix.button import Button
from kivy.uix.boxlayout import BoxLayout
from kivy.properties import StringProperty
from kivy.uix.popup import Popup
from kivy.core.window import Window


class IsprimeApp(App):

    def build(self):

        def avg(lst):
            return sum(lst) / len(lst)

        def get_font(size):
            return str(avg(Window.size) / (10 - size)) + "sp"

        parent = BoxLayout(orientation="vertical")
        number_input = TextInput(
            text=str(random.randint(5, 10 ** 10)), font_size=get_font(3))
        parent.add_widget(number_input)
        btn = Button(text='Is prime?', font_size=get_font(3))

        def callback(instance):
            def isprime(n):
                MAX_TIME = 1
                WRONG_TYPE = "Enter a number"
                MAX = 10 ** 17
                TOO_BIG = "The number must be less than 10^17"
                NO = "No, {} is not prime".format(n)
                YES = "Yes, {} is prime".format(n)
                try:
                    n = int(n)
                except:
                    return WRONG_TYPE
                if n > MAX:
                    return TOO_BIG
                if n in (0, 1):
                    return NO
                if n % 2 == 0 and n != 2:
                    return NO
                upper_limit = int((n ** 0.5) + 1)
                for i in range(3, upper_limit, 2):
                    if n % i == 0:
                        return NO
                return YES

            content = BoxLayout(orientation="vertical")
            content.add_widget(
                Label(text=str(isprime(number_input.text)),
                      font_size=get_font(-15)))
            dismiss = Button(text="OK", font_size=get_font(2))
            content.add_widget(dismiss)
            popup = Popup(title='Is the number prime?',
                          content=content,
                          size_hint=(1, 1),
                          font_size=get_font(2))
            dismiss.bind(on_press=popup.dismiss)
            popup.open()

        btn.bind(on_press=callback)
        parent.add_widget(btn)
        return parent

if __name__ in ('__main__','__android__'):
    IsprimeApp().run()
\$\endgroup\$
  • \$\begingroup\$ Can you please add a short summary about what your app does? \$\endgroup\$ – Simon Forsberg Dec 19 '14 at 0:37
5
\$\begingroup\$

I don't know Kivy, so here are a few general style tips.

First, order you imports. Don't also import everything directly; it's OK to use namespaces. I would probably import as so:

import random

from kivy import app
from kivy.uix import boxlayout, button, popup, textinput
from kivy.core.window import Window

A minor point is that the class name would read much better as IsPrimeApp.

Your get_font is misnamed; it should be get_font_size. The equation is also rather strange; it's infinitely large at size 10, negatively sized at size 11 and you end up very negative sizes to get small fonts. It's also advisable to use formatting over concatenation.

I would suggest something like:

def font_size(size):
    return "{}sp".format(avg(Window.size) / 32 * 1.5 ** size)

I really suggest against wrapping lines like:

number_input = textinput.TextInput(
    text=str(random.randint(5, 10 ** 10)), font_size=font_size(3))

This seems immensely more readable:

number_input = textinput.TextInput(
    text=str(random.randint(5, 10 ** 10)),
    font_size=font_size(3)
)

or any other semantic equivalent (take your pick).

It's potentially better to just reorganize so you don't have to:

default_input = str(random.randint(5, 10 ** 10))
number_input = textinput.TextInput(text=default_input, font_size=font_size(3))

Please don't go naming things parent or btn; give them semantically useful, even if more verbose names.

  • parent → root
  • btn → is_prime_btn
  • callback → check_primality

Move things into separate methods. build does not need to contain a lot.

It seems buttons don't need a bind call; you can just add an on_press in the constructor.

I would consider explicitly making phases where you declare what boxes are where and what contains what.

Your isprime is a "confused" function, and its name doesn't really explain its functionality. I would separate the actual core behaviour (which is buggy for negative numbers) from generating a response. This is more reusable than just adding CONSTANTS, and it's cleaner.

Here's a cleaned up version:

import random

from kivy import app
from kivy.uix import boxlayout, button, popup, textinput
from kivy.core.window import Window

def is_prime(n):
    if n == 2:
        return True

    if n < 2 or not n % 2:
        return False

    sqrt_n = int((n ** 0.5) + 1)

    return all(n % i for i in range(3, sqrt_n, 2))


class IsPrimeApp(app.App):
    def font_size(self, size):
        return "{}sp".format(sum(Window.size) / 64 * 1.5 ** size)

    def check_primality(self, instance):
        def response():
            try:
                n = int(self.number_input.text)
            except:
                 return "Enter a number"

            if n > 10 ** 17:
                return "The number must be less than 10^17"

            elif is_prime(n):
                return "Yes, {} is prime".format(n)

            else:
                return "No, {} is not prime".format(n)

        content = boxlayout.BoxLayout(orientation="vertical")

        response = button.Label(
            text=response(),
            font_size=self.font_size(0)
        )

        question = popup.Popup(
            title='Is the number prime?',
            content=content,
            size_hint=(1, 1),
            font_size=self.font_size(2)
        )

        dismiss = button.Button(
            text="OK",
            font_size=self.font_size(2),
            on_press=question.dismiss
        )

        content.add_widget(response)
        content.add_widget(dismiss)

        question.open()

    def build(self):
        root = boxlayout.BoxLayout(orientation="vertical")

        self.number_input = textinput.TextInput(
            text=str(random.randint(5, 10 ** 10)),
            font_size=self.font_size(3)
        )

        is_prime_btn = button.Button(
            text='Is prime?',
            font_size=self.font_size(3),
            on_press=self.check_primality
        )

        root.add_widget(self.number_input)
        root.add_widget(is_prime_btn)

        return root

if __name__ in ('__main__', '__android__'):
    IsPrimeApp().run()

It's not markedly different, but I would argue that without using a declarative approach to layout there isn't much more you can do. 61 SLOC isn't a lot, and most are dedicated to declaring properties about the buttons, such as strings.

\$\endgroup\$
  • \$\begingroup\$ Why is self.number_input a subclass of self and is_prime_btn is not? \$\endgroup\$ – Caridorc Dec 19 '14 at 12:19
  • 1
    \$\begingroup\$ @Caridorc To allow using number_input from the callback (check_primality). You never change is_prime_btn so there's no reason to keep a reference to it. \$\endgroup\$ – Veedrac Dec 19 '14 at 14:56

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.