Найти - Пользователи
Полная версия: Первая программа, калькулятор, в стиле ООП
Начало » Python для новичков » Первая программа, калькулятор, в стиле ООП
1 2
Brony
Хотелось бы услышать ваше мнение по этому коду:
#! /usr/bin/python

'''Calculator in object oriented style,
__future__ to use true division by '/'
'''

from __future__ import division
from Tkinter import \
Tk, Frame, Entry, Button, \
END, TOP, LEFT, RIGHT

class GUI:

def __init__(self, title='My calc',
width_root=160, height_root=170,
width_num=1, width_math=2,
btn_names='C7894561230.=+-/*'):

self.root = Tk()
self.root.title(title)
self.root.maxsize(width = width_root, height = height_root)
self.root.minsize(width = width_root, height = height_root)

fra = Frame(self.root)
fra_ent = Frame(self.root)
fra_math = Frame(self.root)

fra_num = Frame(fra)
fra_num2 = Frame(fra)
fra_num3 = Frame(fra)
fra_num4 = Frame(fra)

self.entry = Entry(fra_ent)
self.entry.insert(END, '0')

self.entry.pack(side = TOP)

fra_ent.pack(side = TOP)
fra.pack(side = LEFT)
fra_math.pack(side = RIGHT)

fra_num.pack(side = TOP)
fra_num2.pack(side = TOP)
fra_num3.pack(side = TOP)
fra_num4.pack(side = TOP)

strct = {btn_names[0]: (fra_ent, width_math, RIGHT)}
for i in btn_names[1:4]:
strct[i] = (fra_num, width_num, LEFT)
for i in btn_names[4:7]:
strct[i] = (fra_num2, width_num, LEFT)
for i in btn_names[7:10]:
strct[i] = (fra_num3, width_num, LEFT)
for i in btn_names[10:13]:
strct[i] = (fra_num4, width_num, LEFT)
for i in btn_names[13:]:
strct[i] = (fra_math, width_math, TOP)

for name in btn_names:
Button(strct[name][0], text = name,
width = strct[name][1],
command = lambda x = name: self.write(x)
).pack(side = strct[name][2])

def write(self, char):
self.show(char)

def show(self, value):
self.entry.delete(first = 0, last = END)
self.entry.insert(END, value)

def run(self):
self.root.mainloop()


class Calc(GUI):

def __init__(self):
GUI.__init__(self)
self.numb = ''
self.memr = ''
self.result = ''
self.switch = {'=': lambda: self.calculate(),
'C': lambda: self.reset(),
'.': lambda: self.dot(),
'+': lambda: self.operation('+'),
'-': lambda: self.operation('-'),
'*': lambda: self.operation('*'),
'/': lambda: self.operation('/')}

def write(self, char):
if char in self.switch:
self.switch[char]()
else:
self.numb += char
self.show(self.numb)

def reset(self):
self.numb = ''
self.memr = ''
self.result = ''
self.show('0')

def dot(self):
if not self.numb:
# Has not achieved the number before '.'
if self.result:
self.reset()
self.numb = '0.'
self.show(self.numb)
elif '.' in self.numb:
# Repeated, incorrect entry '.'
pass
else:
self.numb += '.'
self.show(self.numb)

def operation(self, char):
'''Arithmetic operations'''
if self.result:
# Use the result of the previous calculation
self.numb = self.result
self.result = ''
self.memr += self.numb + char
self.show(self.numb + char)
self.numb = ''

def calculate(self):
try:
result = eval('%s%s' % (self.memr, self.numb))
except (SyntaxError, ZeroDivisionError):
self.reset()
self.show('ERROR')
else:
self.result = str(result)
self.numb = ''
self.memr = ''
self.show(self.result)

if __name__ == '__main__':
Calc().run()
Ed
В целом неплохо. Но есть вопросы и рекомендации.
- from Tkinter import * это так принято при программировании с использованием Tkinter? Вообще-то import * считается признаком плохого тона - засорение текущего namespace-а непонятно чем с серьезными последствиями.
- font, title, width, height я бы сделал параметрами
- В чем смысл from future import __division__ ?
- почему для кнопок ‘5’ и ‘6’ не используется width=1?
- if __name__ == ‘__main__’: Calc().root.mainloop() я бы не писал в одну строку
- вместо char == ‘+’ or char == ‘-’ or char == ‘/’ or char == ‘*’ я бы использовал char in “+-/*”

Запустите pylint. Ему ваш код сильно не понравился. Его оценка: Your code has been rated at -11.61/10 (previous run: -11.61/10)

PS: Это все при беглом просмотре. Могу найти еще много чего, но сначала это исправьте.
Brony
Ed
- from Tkinter import * это так принято при программировании с использованием Tkinter? Вообще-то import * считается признаком плохого тона - засорение текущего namespace-а непонятно чем с серьезными последствиями.
У Лутца, как и в http://docs.python.org/library/tkinter.html примеры с import *, но это не сложно исправить, что я и сделал.
Ed
- В чем смысл from future import __division__ ?
Истинное деление при отсутствии дополнительной логики.

Спасибо большое за рекомендации. Если есть еще - буду благодарен.
Soteric
Смешивать логику и интерфейс считается неправильным. Должен быть объект Калькулятор, который предоставляет API для выполнения операций над числами. И должен быть объект ГУИ, который имеет ссылку на Калькулятор. Когда пользователь нажимает кнопку, то ГУИ просит Калькулятор выполнить какое-то действие, вызывая его метод, и возвращает пользователю результат. Причем если предполагается, что операция займет продолжительное время, то ее выполнение запускается в отдельном потоке, чтобы не блокировать интерфейс и дать пользователю возможность отменить свое действие. На мой взгляд, это два обязательных пункта, которые выполняются в любой программе с графическим интерфейсом.
Ed
Угу, сейчас получше.
Следующая порция вопросов:
- в чем смысл разделения на 2 класса?
- Может упростить всю эту генерацию кнопок? Там простенькая структурка с фреймами и названиями кнопок просто напрашивается. Они же все одинаковые.
- докстринг модуля поставьте до импортов - pylint перестанет на него ругаться
- может проще отлавливать SyntaxError, который генерит eval, чем проверять синтаксис самому. Мне удалось его завалить тупо набрав / 2 = не говоря уже о 1 / 0
- shebang-а нет. Вы под виндой?

Достаточно :) ?
Brony
Ed
- в чем смысл разделения на 2 класса?
Soteric
Смешивать логику и интерфейс считается неправильным.
Если я правильно понимаю, вы говорите о одном и том же. Исправил свою ошибку переносом метода show из класса Calc в GUI. Теперь Calc обращается к GUI только через наследуемый метод show и переопределенный write.
Упростил генерацию кнопок, но не совсем понимаю зачем. Вроде было более читабельно, где каждая кнопка прописывалась отдельно.
Ed
shebang-а нет. Вы под виндой?
Нет, Debian. Скрипт вне IDLE использовать не предполагается, я даже и не подумал об этом.

Спасибо большое за рекомендации. Если есть еще - буду благодарен.
Brony
Если изменить
def __init__(self):
GUI.__init__(self)
self.numb = ''
self.memr = ''
self.result = ''
на
def __init__(self):
GUI.__init__(self)
self.reset()

def reset(self):
self.numb = ''
self.memr = ''
self.result = ''
pylint ругается: Attribute ‘result’ defined outside __init__. Что плохого в defined outside __init__?
Еще ругань на W: 83:Calc.__init__.<lambda>: Lambda may not be necessary. Это значит что есть еще способ определить словарь выбора?
Ed
Если есть желание разделять UI и логику, то по-моему лучше это делать не путем наследования класса с логикой от класса с UI. Даже с точки зрения здравого смысла это, кхм, вызывает вопросы. Попробуйте вместо наследования просто передавать объект UI в качестве параметра в конструктор вашего калькулятора.
Если есть желание, то можете наваять простенький commandline UI, чтобы проверить достаточен ли интерфейс вашего UI класса. Думаю, вы сильно удивитесь :)
В сторону MVC смотрели?

Насчет кнопок - вы меня неправильно поняли. Чем такая реализация лучше уж то, что было. Я говорил о некой структуре, взглянув на которую можно было понять иерархию UI. Из нее же и генерить все или хотя бы часть виджетов. А при такой реализации читабельность сильно пострадала.
Ed
pylint ругается: Attribute ‘result’ defined outside __init__. Что плохого в defined outside __init__?
то, что когда объект создается у вас этого атрибута еще нет, то есть потенциально можно получить AttributeError при доступе к нему.
PS: Упс, сорри. Это он просто не понимает, что у вас вызов reset в __init__

Еще ругань на W: 83:Calc.__init__.<lambda>: Lambda may not be necessary. Это значит что есть еще способ определить кортеж выбора?
Он абсолютно прав, лямбда там лишняя:
self.switch = {'=': self.calculate,
'C': self.reset,
'.': self.dot
...
Абсолютно идентично тому, что вы написали.
Ed
Еще пара советов:
- лямбду можно заменить на partial из functools, если там простой вызов с определенным параметром.
- то, что при наборе скажем 2 + 1 + 1 после нажатия первой единицы все исчезает мне кажется неправильным. Может сделать просто тупой вывод того, что набирается, а при нажатии на = делать eval? Тогда можно будет и скобки легко реализовать и многое другое. И не нужна будет вся эта лишняя логика с numb, result и memr. Все это выглядит как ненужные костыли. Вы же все равно юзаете eval, лучше и проще будет быть честным до конца :)
This is a "lo-fi" version of our main content. To view the full version with more information, formatting and images, please click here.
Powered by DjangoBB