Efim Kozhemiakin by Efi-fi · Pull Request #11 · PythonAndGoCommunity/PythonHomework

Efi-fi · 2018-12-03T13:36:42Z

Second version, soon will be tests and may be comments

Efi-fi · 2018-12-04T22:07:23Z

Finished development, however, if there are defects, I am ready to fix it.

AlexeiBuzuma · 2018-12-06T09:41:50Z

final_task/calc/main.py

+
+def main():
+    try:
+


Лишняя пустая строка

AlexeiBuzuma · 2018-12-06T09:42:12Z

final_task/calc/main_functions.py

+
+
+def reduction_expression(s):
+


лишняя пустая строка

AlexeiBuzuma · 2018-12-06T09:44:08Z

final_task/calc/main_functions.py

+from calc.math_functions import decide_func
+
+
+def reduction_expression(s):


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

AlexeiBuzuma · 2018-12-06T09:45:26Z

final_task/calc/main_functions.py

@@ -0,0 +1,114 @@
+import calc.other_functions as o_f


Человеку, который видит этот код впервые, сложно понять, что значит o_f. Я думаю, в данном случае лучше оставить other_functions

AlexeiBuzuma · 2018-12-06T09:48:17Z

final_task/calc/main_functions.py

+    return lis
+
+
+def compare(lis):


Из названия функции и названия аргументов нельзя получить информацию, что эта функция делает и что в эту функцию нужно отправить в качестве аргумента. Отсутствие документации усложняет ситуацию. Я бы дал более осмысленное название функции и ее аргументам.

AlexeiBuzuma · 2018-12-06T09:51:37Z

final_task/calc/main_functions.py

+
+def compare(lis):
+    i = 0
+    while i < len(lis)-1:


С-style итерирование по коллекции через индексы крайне редко используется в Python. В Python есть замечательный цикл for, который позволяет удобным образом итерироваться по данным в коллекции без использования индексов.

for item in lis: # do smt pass

А если мне не нужно последний элемент проверять, то брать срез?
for item in lis[:-1]:
...

Или чтоб получать индексы использовать range:
for item in range(len(lis)-1):
...

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

for index, element in enumerate(lis[:-1]): pass

AlexeiBuzuma · 2018-12-06T09:53:39Z

final_task/calc/main_functions.py

+    return st_nums[0]
+
+
+def verify(s, i, st_nums, st_ops):


Очень сложно читать код, где названия переменных не несут в себе смысла. Как минимум я бы переименовал переменные s и i

AlexeiBuzuma · 2018-12-06T09:54:20Z

final_task/calc/main_functions.py

+        i += 1
+
+    if len(st_nums) > 1 or len(st_ops):
+        print('ERROR: not necessary operation')


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

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

В данном случае вместо print и выхода из программы можно "бросить" исключение, например:

class InvalidExpressionError(Exception): pass if len(st_nums) > 1 or len(st_ops): raise InvalidExpressionError

А на самом верхнем уровне, где вызывается функция main, можно эти исключения обрабатывать и корректно завершать программу:

try: main() except Excpetion as e: print(f"ERROR: {e})

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

PS: код приведен для примера, написан "на коленке" и без проверки того, что он работает.

AlexeiBuzuma · 2018-12-06T09:57:52Z

final_task/calc/math_functions.py

+        print('ERROR: so many arguments')
+        exit()
+
+    elif func == 'abs':


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

functions_mapping = { "sin": math.sin, ... }

Очень хорошая вещь этот маппинг, жалко раньше не знал.

AlexeiBuzuma · 2018-12-06T09:58:15Z

final_task/calc/math_functions.py

+
+def decide_func(func, ready_args):
+    if len(ready_args) == 0:
+        print('ERROR: no necessary arguments ')


В данном случае будет уместно использовать исключения.

AlexeiBuzuma · 2018-12-06T09:58:21Z

final_task/calc/math_functions.py

+        exit()
+
+    elif len(ready_args) > 2:
+        print('ERROR: so many arguments')


В данном случае будет уместно использовать исключения.

AlexeiBuzuma · 2018-12-06T09:58:30Z

final_task/calc/math_functions.py

+        return log(ready_args[0], ready_args[1])
+
+    elif len(ready_args) < 2:
+        print('ERROR: no necessary arguments or our function "' + s[i] + '"')


В данном случае будет уместно использовать исключения.

AlexeiBuzuma · 2018-12-06T09:58:37Z

final_task/calc/math_functions.py

+        return ldexp(ready_args[0], ready_args[1])
+
+    else:
+        print('ERROR: not find function "' + func + '"')


В данном случае будет уместно использовать исключения.

AlexeiBuzuma · 2018-12-06T10:00:11Z

final_task/calc/other_functions.py

+    i = 0
+    while i < len(s):
+
+        if 96 < ord(s[i]) < 122:


Магические константы. Что обозначают 96 и 122?
Как минимум можно занести эти значения в переменные с читаемым именем.

AlexeiBuzuma · 2018-12-06T10:00:46Z

final_task/calc/other_functions.py

+    unit_fun = ''
+    s += ' '
+    i = 0
+    while i < len(s):


Итерирование как в языке С. В даннос случае лучше использовать цикл for

Почему for лучше? Или его просто принято использовать?

Python высокоуровневый язык программирования. В нем есть много концепций, которые облегчают жизнь программистам. В том числе и цикл for
Цикл for позволяет абстрагироваться от индексов и поиска элемента в коллекции по этому индексу, а просто работать с итератором, который возвращает следующие значение.
В большинстве случаев, индексы программисту не нужны, так как часто используется обычный последовательный проход по данным.
Идея в простоте. Индексы прото не нужны.

AlexeiBuzuma · 2018-12-06T10:01:10Z

final_task/calc/other_functions.py

+            unit_fun += s[i]
+            unit_fun = verify_pi_e(unit_fun, lis)
+
+        elif (47 < ord(s[i]) < 58) or s[i] == '.':


Магические константы.

AlexeiBuzuma · 2018-12-06T10:02:13Z

final_task/calc/other_functions.py

+
+def verify_pi_e(unit_func, lis):
+    if unit_func == 'e':
+        lis.append(2.718281828459045)


В стандартной бибилиотеке math есть эти значения. Не нужно их набирать самостоятельно.

Понял, но хотел как лучше: быстрее и меньше памяти.

AlexeiBuzuma · 2018-12-06T10:02:55Z

final_task/calc/other_functions.py

+        exit()
+
+
+def verify_pi_e(unit_func, lis):


В библиотеке math существует переменная tau. Учитывается ли она в этом решении?

tau = 2*pi, решил её не включать, так как ей никто не пользуется и её не сложно через пи выразить.
Если без неё никак, то добавить можно)

Есть конкретные требования в задании:

All functions and constants from standard python module math (trigonometry, logarithms, etc.).

Так, точно)

AlexeiBuzuma · 2018-12-06T10:03:54Z

final_task/calc/other_functions.py

+    return lis
+
+
+def prior(op):


Название функции должно быть глаголом.

Можно поинтересоваться почему, специально в PEP8 заглянул, там такого не написано.

Функция -- это действие, а действие описывает глагол, вот и все :)
Поясню свой комментарий.
Имя функции не должно состоять из одного глагола. Название функции должно описывать какое-то действие.
в данном случае вместо prior можно дать название get_priority

Благодарю за уточнение.

AlexeiBuzuma · 2018-12-06T10:04:19Z

final_task/calc/other_functions.py

+        return 5
+
+
+def bin_operate(a, b, op):


Название функции должно быть глаголом.

AlexeiBuzuma · 2018-12-06T10:07:49Z

final_task/calc/other_functions.py

+
+def additions(lis):
+    i = 0
+    while i < len(lis)-1:


Лучше использовать цикл for для генерации новой коллекции, чем видоизменять колекцию, которая передана как аргумент функции, прямо в момент итерации по этой коллекции

Привычка после С++, буду пробовать исправляться.

AlexeiBuzuma · 2018-12-06T10:10:26Z

final_task/calc/unit_tests.py

+        formated_expression = get_args(['(', '(', 2, ')', ',', 4, ',', '(', ')', ')'])
+        self.assertEqual(formated_expression, [['(', 2, ')'], [4], ['(', ')']])
+
+    def test14_ga3(self):


Не нужно использовать нумерацию тест кейсов. Достаточно будет дать им читаемое имя.

AlexeiBuzuma · 2018-12-06T10:12:15Z

final_task/calc/main_functions.py

+    st_ops = []
+    i = 0
+
+    while i < len(s):


Цикл for

AlexeiBuzuma · 2018-12-06T10:14:02Z

final_task/calc/math_functions.py

@@ -0,0 +1,137 @@
+from math import *


Импортирование через звездочку может привести к конфликту имен или просто запутать программиста.
Я думаю, будет лучше просто сделать импорт библиотеки math

import math

Efi-fi · 2018-12-19T11:01:49Z

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

Efi-fi · 2018-12-19T14:19:35Z

Можно конечно менять на for, но для этого нужно менять уже и логику, а времени уже нету.

Efi-fi added 5 commits December 3, 2018 16:33

working version 2.0 - -

ac11952

adding unit_tests

01846e5

fix unit_tests

415dfb9

fix unit_tests: delete running

c312ad6

add some unittests

e0325f1

Efi-fi changed the title ~~[WIP] Efim Alekseevich~~ Efim Kozhemiakin Dec 4, 2018

AlexeiBuzuma reviewed Dec 6, 2018

View reviewed changes

final_task/calc/main.py Outdated

def main():

try:

Copy link
Copy Markdown

AlexeiBuzuma Dec 6, 2018

Choose a reason for hiding this comment

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

Лишняя пустая строка

AlexeiBuzuma reviewed Dec 6, 2018

View reviewed changes

final_task/calc/main_functions.py

def reduction_expression(s):

Copy link
Copy Markdown

AlexeiBuzuma Dec 6, 2018

Choose a reason for hiding this comment

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

лишняя пустая строка

AlexeiBuzuma reviewed Dec 6, 2018

View reviewed changes

final_task/calc/main_functions.py

st_ops = []

i = 0

while i < len(s):

Copy link
Copy Markdown

AlexeiBuzuma Dec 6, 2018

Choose a reason for hiding this comment

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

Цикл for

AlexeiBuzuma reviewed Dec 6, 2018

View reviewed changes

Efi-fi added 2 commits December 18, 2018 15:35

correction of defects

ce8c04f

fiX bags in tests

b842ccf

PythonAndGoCommunity added the Code freeze label Dec 20, 2018

		from calc.math_functions import decide_func


		def reduction_expression(s):

Conversation

Efi-fi commented Dec 3, 2018

Uh oh!

Efi-fi commented Dec 4, 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!

Efi-fi Dec 6, 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!

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!

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!

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

Efi-fi Dec 6, 2018 •

edited

Loading