Nikita Kaleko #21

Shalynishka · 2018-12-14T18:09:06Z

No description provided.

v 0.3a

Distribution package

adding description of some methods

AlexeiBuzuma · 2018-12-15T14:57:44Z

final_task/pycalc/pycalc.py

+    func = {'round': (round, 4), 'abs': (abs, 4)}
+
+    users_mod = {}
+    bin_op = {'+': (operator.add, 1), '-': (operator.sub, 1), '*': (operator.mul, 2),


Возможно тут будет удобно использовать namedtuple для хранения данных вроде (operator.add, 1)

AlexeiBuzuma · 2018-12-15T16:06:29Z

final_task/pycalc/pycalc.py

+        eval_not(self) - evaluates RPE using eval_list as source
+        check_num(self, temp_num) - converts str to int or float and store num in eval_list
+    """
+    const = {'pi': math.pi, 'e': math.e, 'q': math.pi * math.e}


Можно достать все числа из модуля красивым способом

import math import numbers numbers = {attr: getattr(math, attr) for attr in dir(math) if isinstance(getattr(math, attr), numbers.Number)} print(numbers) {'e': 2.718281828459045, 'inf': inf, 'nan': nan, 'pi': 3.141592653589793, 'tau': 6.283185307179586}

Заменил, но немножко иначе.

AlexeiBuzuma · 2018-12-15T16:08:34Z

final_task/pycalc/pycalc.py

+    unary = True
+    implicit_mul = False    # флаг на случай неявного умножения "(2+3)4"
+
+    # лимит на длину буквенного выражения. Если его превысить -> raise ValueError


В данном случае это не критично, но лучше писать комментарии в коде на английском языке, потому что английский поймут все, а русский не факт (мой комментарий тоже должен был быть на английском :) )

AlexeiBuzuma · 2018-12-15T16:11:53Z

final_task/pycalc/pycalc.py

+def calculate():
+    re = {' + ': '+', ' * ': '*', ', ': ',', ' - ': '-', '\'': '', '"': ''}
+    try:
+        parser = argparse.ArgumentParser(description='Pure-python command-line calculator.')


Функция calculate содержит логику по настройке интерфейса консольной утилиты. Это не ее ответственность, возможно нужно вынести логику по созданию парсера в отдельную функцию.

AlexeiBuzuma · 2018-12-15T16:16:16Z

final_task/pycalc/pycalc.py

+        parser.add_argument('EXPRESSION', help='Expression string to evaluate')
+        parser.add_argument('-m', '--use-modules', action='store', nargs='*',
+                            dest='user', help='Using your own module', default=None)
+        pr = parser.parse_args().__dict__


В данном случае использовать магическую переменную __dict__ не нужно. Можно просто получать достп к атрибутам через точку

args = parser.parse_args() args.EXPRESSION

AlexeiBuzuma · 2018-12-15T16:21:32Z

final_task/pycalc/pycalc.py

+            raise ValueError('incorrect symbols!')
+        else:
+            s = s.replace('//', '$')
+            s = s.replace('epi', 'q')


Насколько я понимаю, это частное решение для реализации неявного умножения. Если использовать неявное умножение с какой-либо другой константой, то оно работать не будет, например tau и pi.
Реализовывать эту часть функционала по-другому необязательно, просто нужно понимать, что это dirty hack

Конкретная замена символов "//" на "$" - именно читы, чтобы не читать два деления. А вот со вторым уже нет никакой разницы, т.к. найдя константу в подстроке, подключится возможность неявного умножения. Это был костыль в самом начале, когда об обработке неявного умножения речи не шло, но убрать надо. :)

AlexeiBuzuma · 2018-12-15T16:24:12Z

final_task/pycalc/pycalc.py

+
+class Calc:
+    """
+    Pretty class for cmd evaluating


Эта строчка документация не несет никакой полезной информации. Нет понимания того, для чего предназначен класс Calc

AlexeiBuzuma · 2018-12-15T16:26:55Z

final_task/pycalc/pycalc.py

+
+    cmp_op = []
+
+    def __init__(self, users: list):


В моем понимании users -- это набор объектов, которые представляют людей, которые пользуются этим программным решением :)

Возможно нужно придумать другое название переменной, которое будет четко описывать ее предназначение.

AlexeiBuzuma · 2018-12-15T16:28:14Z

final_task/pycalc/pycalc.py

+                else:
+                    self.users_mod[foo] = dir(foo)
+
+    def my_eval(self, st: str):


Такая же ситуация. Не совсем понятно, что значит название функции my_eval и для чего она предназначена. Особенно сложно понять, что значит аргумент st.

Возможно было бы хорошо добавить документацию к функции

AlexeiBuzuma · 2018-12-15T16:31:54Z

final_task/pycalc/pycalc.py

+                    foo = importlib.util.module_from_spec(spec)
+                    spec.loader.exec_module(foo)
+                except:
+                    continue


Я думаю, что ошибки такого рода не должны замалчиваться.
Если программа не смогла найти модуль, который пользователь передал в утилиту, то мы должны явно сказать пользователю, что тут возникла ошибка. Модуль не найден и программа не может выполнять вычисления используя несуществующий модуль.

AlexeiBuzuma · 2018-12-15T16:36:31Z

final_task/pycalc/pycalc.py

+                self.make_note(string)              # сборка ОПЗ
+                res.insert(0, self.eval_note())   # вычисление ОПЗ
+                self.eval_list = []
+            for op in self.cmp_op:


Я думаю, в таком случае лучше не скупиться на символы и написать хотя бы вот так

for cmp_operator in self.cpm_operators: ...

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

AlexeiBuzuma · 2018-12-15T16:37:17Z

final_task/pycalc/pycalc.py

+        self.implicit_mul = False
+        egg = ''       # содержит операцию
+        temp_num = ''  # содержит операнд
+        for c in st:


Желательно не использовать в коде переменные, название которых состоит из одного символа. Это всегда плохо читается.

AlexeiBuzuma · 2018-12-15T16:39:10Z

final_task/pycalc/pycalc.py

+    def make_note(self, st: str, impl=False):
+        self.unary = True
+        self.implicit_mul = False
+        egg = ''       # содержит операцию


Почему переменная, которая должна содержать операцию, называется яйцо и инициализируется пустой строкой?)

AlexeiBuzuma · 2018-12-15T16:40:01Z

final_task/pycalc/pycalc.py

+                egg += c  # формируем строку с буквами - sin/pi/epi и тд. Из этого потом сформируем функции или const
+
+                # попытка вытащить текущий egg из пользовательского модуля
+                for u, d in self.users_mod.items():


Та же ситуация. Переменные из одного символа очень сложно читаются.

AlexeiBuzuma · 2018-12-15T16:42:31Z

final_task/pycalc/pycalc.py

+
+
+def calculate():
+    re = {' + ': '+', ' * ': '*', ', ': ',', ' - ': '-', '\'': '', '"': ''}


Первое, что приходит в голову, при виде переменной re -- это что-то связанное с регулярными выражениями (есть такой модуль re). Но в данном случае это что-то другое. Нужно другое название для переменной.

AlexeiBuzuma

Есть очень много проблем с именованием переменных.

Style code updated.

Algorithm has been updated.

Defenition of similar function(log vs log10)

Shalynishka added 9 commits December 14, 2018 21:00

Create pycalc.py

28af816

v 0.3a

Add files via upload

4ec8dbf

Distribution package

Create pycalc.py

7612bdc

Create __init__.py

5b9787c

Update setup.py

f199540

Update pycalc.py

fae9a0f

Delete pycalc.py

b752c81

Update pycalc.py

1b6c83c

Update pycalc.py

f6fb8a0

adding description of some methods

AlexeiBuzuma reviewed Dec 15, 2018

View reviewed changes

Update setup.py

aa85e0e

AlexeiBuzuma reviewed Dec 15, 2018

View reviewed changes

AlexeiBuzuma suggested changes Dec 15, 2018

View reviewed changes

Shalynishka added 4 commits December 16, 2018 22:38

Update setup.py

a7d6749

Update pycalc.py

4ed2fc7

Style code updated.

Update pycalc.py

596dd4a

Algorithm has been updated.

Delete pycalc.tar.gz

7b0a5ff

Shalynishka added 3 commits December 17, 2018 01:30

Update pycalc.py

5e099eb

Update pycalc.py

739a6d7

Defenition of similar function(log vs log10)

Update pycalc.py

d30054b

Shalynishka changed the title ~~[WIP] Nikita Kaleko~~ Nikita Kaleko Dec 19, 2018

PythonAndGoCommunity added the Code freeze label Dec 20, 2018



		def calculate():
		re = {' + ': '+', ' * ': '*', ', ': ',', ' - ': '-', '\'': '', '"': ''}

Nikita Kaleko #21

Are you sure you want to change the base?

Nikita Kaleko #21

Uh oh!

Conversation

Shalynishka commented Dec 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexeiBuzuma Dec 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexeiBuzuma left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AlexeiBuzuma Dec 15, 2018 •

edited

Loading