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()
-
\$\begingroup\$ Can you please add a short summary about what your app does? \$\endgroup\$Simon Forsberg– Simon Forsberg2014年12月19日 00:37:35 +00:00Commented Dec 19, 2014 at 0:37
1 Answer 1
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.
-
\$\begingroup\$ Why is
self.number_input
a subclass of self andis_prime_btn
is not? \$\endgroup\$Caridorc– Caridorc2014年12月19日 12:19:18 +00:00Commented Dec 19, 2014 at 12:19 -
1\$\begingroup\$ @Caridorc To allow using
number_input
from the callback (check_primality
). You never changeis_prime_btn
so there's no reason to keep a reference to it. \$\endgroup\$Veedrac– Veedrac2014年12月19日 14:56:55 +00:00Commented Dec 19, 2014 at 14:56