[WIP] Nick Dubovik #20
Conversation
| import argparse | ||
| import sys | ||
|
|
||
| OPERATORS = {'+': (2, operator.add), '-': (2, operator.sub), |
There was a problem hiding this comment.
В данном случае будет лучше использовать namedtuple для хранения функции и других параметров, которые связаны с операторами
| '!=': (1, operator.ne), '>=': (1, operator.ge), | ||
| '==': (1, operator.eq), '?': (4, operator.neg)} | ||
|
|
||
| STLO = {'True': True, 'False': False} |
There was a problem hiding this comment.
- название некоторых глобальных переменных не совсем понятны. Что означает
STLO? - переменная
FUNCTIONпо факту является не функцией, а маппингом нескольких функций на их названия. Лучше будет этой переменной названиеFUNCTIONS_MAPPING - символы всех цифр и букв можно найти в модуле
string
import string
string.ascii_letters
'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'
string.digits
'0123456789'| STOP = '<+->/!=*^%' | ||
|
|
||
| version = "1.2.1" | ||
| global module |
There was a problem hiding this comment.
Зачем в глоабльной секции кода использовать ключевое слово global?
| metavar='MODULE') | ||
| parser.add_argument('string', type=str, default='', help='Expression string to evaluate', | ||
| metavar='EXPRESSION') | ||
| # subparsers = parser.add_subparsers (dest = 'command', |
There was a problem hiding this comment.
Желательно, чтобы закомментированный код не появлялся в истории коммитов, часто это просто мусор, который мешает читать код.
| def check_module(module_name): | ||
| # Checks if the module can be imported without actually importing it | ||
| module_spec = importlib.util.find_spec(module_name) | ||
| if module_spec is None: |
There was a problem hiding this comment.
В данном случае это бессмысленная конструкция. if можно опустить и просто написать вот так:
return importlib.util.find_spec(module_name)|
|
||
|
|
||
| def check_module(module_name): | ||
| # Checks if the module can be imported without actually importing it |
There was a problem hiding this comment.
документацию к функциям, классам и т.д. обычно оборачивают в тройные кавычки:
def check_module(module_name):
"""Checks if the module can be imported without actually importing it"""| return module | ||
|
|
||
|
|
||
| def sum(module, sst): |
There was a problem hiding this comment.
Перекрытие built-in функции sum. Лучше для этой функции придумать более конкретное имя, как минимум не перекрывающее встроенные в Python функции.
|
|
||
| def sum(module, sst): | ||
| # Reading and token allocation-------------------------------------------------------------------- | ||
| def parse(sst): |
There was a problem hiding this comment.
Очень большая функция с большим количеством переменных, которые названы без какой-то смысловой нагрузки.
- pr
- lev
- j
- s
- sst
- n
- i
- cc
и так далее
Желательно, чтобы названия переменных несли какой-то смысл и помогали программисту понимать код.
| if sst[j - 1] in '/*%^><=' and sst[j + 1] in '/*%^><=': | ||
| raise Exception('Пробел между знаками') | ||
| if sst[j - 1] in STNUMBER and sst[j + 1] in STNUMBER: | ||
| raise Exception('Пробел') |
There was a problem hiding this comment.
Лучше использовать английский язык для текста ошибки (как минимум лучше не использовать два разных языка).
| if pr != lev: | ||
| raise Exception('Unequal number of brackets') | ||
|
|
||
| # conversion to Polish notation------------------------------------------------------------------------------ |
There was a problem hiding this comment.
Странный комментарий, который не понятно к чему относится и не понятно, что он должен пояснить.
| description='A program to calculate complex mathematical expressions with support for custom libraries.', | ||
| scripts={"pycalc.py"}, | ||
| packages=find_packages(), | ||
| entry_points={'console_scripts': ['pycalc=pycalc.__main__']}, install_requires=['numpy'] |
There was a problem hiding this comment.
numpy ???
Зачем в данном решении нужен numpy? :D
AlexeiBuzuma
left a comment
AlexeiBuzuma
left a comment
There was a problem hiding this comment.
- Есть большое количество проблем с именованием переменных.
- Нужно сделать декомпозицию кода, так как функции очень большие и разобраться в них очень-очень сложно.
PythonAndGoCommunity
added
the
duplicate
3 participants
pycalc