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()
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Dec 18, 2014 at 20:11
\$\endgroup\$
1
  • \$\begingroup\$ Can you please add a short summary about what your app does? \$\endgroup\$ Commented Dec 19, 2014 at 0:37

1 Answer 1

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.

answered Dec 19, 2014 at 1:40
\$\endgroup\$
2
  • \$\begingroup\$ Why is self.number_input a subclass of self and is_prime_btn is not? \$\endgroup\$ Commented Dec 19, 2014 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\$ Commented Dec 19, 2014 at 14:56

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.