Skip to content

Aliaksei Milto #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open

Conversation

AlexM2000
Copy link

pep8

@@ -0,0 +1,259 @@
import math as m

func_dictionary = {'sin': m.sin,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Подход в лоб. Все функции из модуля можно легко получить динамически.
Вот один из примеров, как можно в рантайме получить абсолютно все вызываемые объекты из модуля

math_functions = [getattr(math, attr) for attr in dir(math) if callable(getattr(math, attr))]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я знаю, но мой калькулятор при работе с функциями завязан именно на словаре, а здесь список.
Если есть какой-нибудь эффективный способ сделать именно такой словарь, я исправлю это.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dict([(attr, getattr(math, attr)) for attr in dir(math) if callable(getattr(math, attr))])

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dict([(attr, getattr(math, attr)) for attr in dir(math) if callable(getattr(math, attr))])

Оу
Спасибо)

return index


def wrapper(Func, Args):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Совсем не очевидный подход.
0) Название этой функции никак не передает то, что в ней происходит.

  1. Намного лучше будет обрабатывать все исключения на самом высоком уровне, а не задавливать эти исключения всюду таким образом
  2. В данном случае, если произошло исключение, то программа продолжит свое исполнение, хотя скорее всего приложение должно аварийно заверишть работу

'+', '-', '*', '/', '%', '^',
'>', '<', '=', ')', '!', ','
):
raise ValueError("""ERROR: invalid argument on position {}""".format(index))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

в данном случае нужны одинарные кавычки, не тройные

elif index < len(eval_string) and eval_string[index] == '(':
variable, index = get_bracket(eval_string, index)
elif index < len(eval_string) and eval_string[index].isalpha():
str = ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

название переменной пересекается с классом str из built-in
советую придумать другое имя для этой переменной

while index < len(eval_string) and (eval_string[index].isalpha() or eval_string[index].isdigit()):
str += eval_string[index]
index += 1
if (str == 'pi'):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

советую использовать такой же подход, как и с функциями. Маппинг названия константы и значения самой константы.

if (znak == '*'):
mult *= num1
elif (znak == '/'):
mult /= num1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему в функции под названием "умножение" может происходить деление или возведение в степень? :)

Copy link
Author

@AlexM2000 AlexM2000 Apr 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Потому что операции одного приоритета на мой взгляд ;)
И возведения в степень там нет, кстати

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тогда все равно не могу понять, почему функция называется multiply

return mult, index


def sum(eval_string, index):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

название пересекается с built-in



def sum(eval_string, index):
sum, num1 = 0, 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а тут уже название пересекается с global



def solve_equality(eval_string, index):
num1, num2, znak = 0, 0, ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

транслит

index += 1
else:
break
return (float(num), index)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Советую для таких случаев импользовать namedtuple
Зачастую это может знаительно упростить понимание кода

raise ValueError('ERROR: no closing bracket')


def error(args):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

эта функция не нужна. Ошибки можно и нужно генерировать прямо там, где они происходят

@AlexM2000 AlexM2000 changed the title [WIP] Aliaksei Milto Aliaksei Milto May 10, 2019
@AlexM2000 AlexM2000 changed the title Aliaksei Milto [WIP] Aliaksei Milto May 10, 2019
@AlexM2000 AlexM2000 changed the title [WIP] Aliaksei Milto Aliaksei Milto May 15, 2019
while index < len(eval_string) and (eval_string[index].isalpha() or eval_string[index].isdigit()):
math_object += eval_string[index]
index += 1
if (math_object == 'pi'):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Подход в лоб. Константы можно получать динамически из модуля.

@AlexeiBuzuma
Copy link
Collaborator

  1. Сообщения в гите должны содержать информацию о том, что сделано в этом коммите. Сообщение Устарел не несет никакой полезной информации.
  2. Артефакты выполнения утилит или собранные пакеты не хранят в репозитории. Файл pycalc-0.0.4.tar.gz и все, что связано с созданием пакета не нужно добавлять в репозиторий.
  3. Зачастую функции очень тяжело читать из-за их размера и количества логики, которая содержиться в этих функциях. Советую в будущем больше внимание уделять на декомпозицию. Много маленьких функций намного лучше, чем несколько больших. Возможно использование ООП помогло бы сделать код более читаемым.

+ комментарии выше

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants