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

Badriev A. (learn-homework-1) #141

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

Conversation

badrievad
Copy link

No description provided.

Copy link

@krepysh krepysh left a comment

Choose a reason for hiding this comment

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

Отлично, но иногда переусложняешь простые вещи. Всегда держи в голове вопрос, а могу ли я это сделать проще?

1_if1.py Outdated
dict_have_to_do = {tuple(range(1, 3)): 'Вам нужно учиться в детском саду',
tuple(range(3, 18)): 'Вам нужно учиться в школе',
tuple(range(18, 21)): 'Вам нужно учиться в ВУЗе',
tuple(range(21, 80)): 'Вам нужно работать'}
Copy link

Choose a reason for hiding this comment

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

когда ты оборачиваешь range в tuple, ты заставляешь программу хранить в память все 60 чисел, в несортированном тупле. Это и памяти много занимает, и перебор он делает последовательно по всем числам в списках.

По крайней мере оставь просто range, должно работать.

Но вообще задача была про сделать цепочку if.
C одной стороны у тебя классный, расширяемый подход. С другой стороны цепочка if понятнее, и работает быстрее. Если тебе не нужно вытаскивать произвольный список правил из БД, то я бы сказал что эта абстрактность тут ни к чему - она только усложняет код.

2_if2.py Outdated
и выводя на экран результаты

"""


def strings(str1, str2):
if isinstance(str1, str) and isinstance(str2, str):
Copy link

Choose a reason for hiding this comment

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

А можно сделать код менее вложенным? Вложенный условия сложно понимать. Совершенно не вижу причины для трех уровней вложенности тут.

3_for.py Outdated

def sum_sales_for_each_product(lsts):
for lst in lsts:
print(f'Cуммарное количество продаж товара: "{lst["product"]}" составляет {sum(lst["items_sold"])} шт.')
Copy link

Choose a reason for hiding this comment

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

print наружу из функции, тут делаем return

3_for.py Outdated

def avg_sales_for_each_product(lsts):
for lst in lsts:
print(f'Среднее количество продаж товара: "{lst["product"]}" составляет {round(np.average(lst["items_sold"]))} шт.')
Copy link

Choose a reason for hiding this comment

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

Попробуй решить без numpy. Использование нампи не дает большой выгоды тут, и в продакшен коде * такое использовать не принято

3_for.py Outdated


def avg_sales_for_all_product(lsts):
return f'Среднее количество продаж товаров: "{", ".join([lst["product"] for lst in lsts])}" составляет {round(np.average([np.average(lst["items_sold"]) for lst in lsts]))} шт.'
Copy link

Choose a reason for hiding this comment

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

Кажется среднее средних не тоже самое что просили посчитать в задаче.

"""
pass

while True:
Copy link

Choose a reason for hiding this comment

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

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

7_exception2.py Outdated
try:
price = abs(float(price))
except (ValueError, TypeError):
return f'Произошла ошибка {sys.exc_info()[0]}. Проверьте корректность введенной цены товара {price=}.'
Copy link

Choose a reason for hiding this comment

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

Можно проще получать текущее исключение, в except блоке добавляешь as ex, и используешь переменную ex

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

Successfully merging this pull request may close these issues.

2 participants