Skip to content
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

Andrei Trukhan #37

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

Andrei Trukhan #37

wants to merge 73 commits into from

Conversation

AngrySamaritan
Copy link

@AngrySamaritan AngrySamaritan commented May 21, 2019

Release version.

@AngrySamaritan AngrySamaritan changed the title [WIP] Andrei Trukhan Andrei Trukhan May 23, 2019
@AngrySamaritan
Copy link
Author

test_main.py полностью отрабатывает на локальной машине. Не понимаю, почему не запускается тут.

@@ -0,0 +1,105 @@
from pycalc.main import Stack
from pycalc.consts import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

Импорт через звездочку -- это зло

from pycalc.consts import *


def check_mistakes(expression, functions={}, mode=0):
Copy link
Collaborator

Choose a reason for hiding this comment

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

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


if len(expression) == 0:
if mode == 0:
return "ERROR: empty expression"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Для этих целей нужно использовать исключения, а не возвращаемые значения

return result


def separate(expression): # separates expression to logical parts
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. Сообщения коммитов в гите должны нести какой-то смысл изменения, которое было сделано в этом коммите. Сообщение Add files via upload не несет особого смысла, а идущие подряд 5 коммитов с таким же сообщением не делают ситуацию лучше.
  2. Артефакты выполнения утилит или собранные пакеты не хранят в репозитории. Файл pycalc-0.1.tar.gz и все, что связано с созданием пакета не нужно добавлять в репозиторий. Такая же ситуация с файлами, которые содержат байткод (.pyc). Их не нужно добавлять в репозиторий
  3. В Python есть замечательный механизм исключений, который позволяет удобным образом обрабатывать ошибки. Ошибки в качестве возвращаемого значения в функциях -- это плохое архитектурное решение.
  4. Зачастую функции очень тяжело читать из-за их размера и количества логики, которая содержиться в этих функциях. Советую в будущем больше внимание уделять на декомпозицию. Много маленьких функций намного лучше, чем несколько больших. Возможно использование ООП помогло бы сделать код более читаемым.
  5. Названия глобальных переменных пишут большими буквами через нижнее подчеркивание. например MATH_CONST
  6. В данном проекте есть использование глобальных переменных на запись, что делает код менее читаемым и может привести к большому количеству ошибок.

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.

3 participants