-
Notifications
You must be signed in to change notification settings - Fork 444
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
Alexandra Poturaeva #137
base: master
Are you sure you want to change the base?
Alexandra Poturaeva #137
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В целом гуд, немного переусложнила с исключениями, и пара мелких правок помимо этого
@@ -121,3 +121,7 @@ dmypy.json | |||
|
|||
# Pyre type checker | |||
.pyre/ | |||
|
|||
.idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше вносить это в .gitignore и не коммитить
1_if1.py
Outdated
работы функции в переменную | ||
* Вывести содержимое переменной на экран | ||
|
||
""" | ||
|
||
from typing import Optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯👌
2_if2.py
Outdated
и выводя на экран результаты | ||
|
||
""" | ||
|
||
|
||
def compare_strings(string_1: str, string_2: str) -> int: | ||
if not all([isinstance(string_1, str), isinstance(string_2, str)]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тк их всего 2, лучше заменить all обычный or
3_for.py
Outdated
total_sale_times = 0 | ||
|
||
print('1-2. Суммарное/среднее количество продаж по товарам:') | ||
for product in sales_info: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
разбей решение на несколько функций
print только снаружи функций оставь
else: | ||
return price - (price * discount / 100) | ||
except ValueError: | ||
traceback_split = traceback.format_exc().split() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🫨
if var in traceback_split: | ||
return f'Неверное значение переменной {var}' | ||
except TypeError: | ||
traceback_split = traceback.format_exc().split() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Очень интересная, но и неожиданная идея, обрабатывать так исключения. Этого не должно быть в бизнес коде (а эта функция это бизнес код).
|
||
|
||
def discounted(price, discount, max_discount=20): | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В try должно быть как можно меньше кода, здесь ты пытаешься перехватить всех , и потом в обработке исключения пытаешься понять что же все таки происходило.
Сделай по отдельному try-catch на price, discount & max_discount
update.message.reply_text(text) | ||
def check_current_planet_constellation(update, context): | ||
current_date = date.today() | ||
ephem_planets = { |
There was a problem hiding this 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
from typing import Optional | ||
|
||
|
||
def define_occupation_by_age(age: int) -> Optional[str]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно придумать разумное "дефолтное значение" и сделать функцию -> str.
Обычно это чуть упрощает код.
if user_age < 0 or user_age > 120: | ||
print('Возраст введён некорректно, попробуйте ещё раз') | ||
else: | ||
user_expected_occupation = define_occupation_by_age(user_age) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
здесь все равно ты не обрабатывешь None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В целом гуд, немного переусложнила с исключениями, и пара мелких правок помимо этого
There was a problem hiding this 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
return 'учиться в ВУЗе' | ||
elif 24 <= age < 65: | ||
return 'работать' | ||
elif 65 <= age <= 120: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C точки зрения типизации, функция все еще возвращает Optional, потому что при возрасте больше 120 вернет None
Убери проверку на верхнюю границу (и нижнюю).
Сигнатура функции (age: int) -> str: означает что такая функция для любого инта вернет некоторую строку.
Так как ты проверяешь возраст на интервал 0-120 в другом месте, ничего страшного не случится, если ты будешь возвращать "учиться в детском саду" на отрицательный возраст, но это чуть упрощает жизнь тем кто будет использовать эту функцию - не надо помнить о том что она может вернуть None
# Вариант 1 (получение информации о переменной, которая передана неверно, из traceback) | ||
def discounted(price, discount, max_discount=20): | ||
try: | ||
price = float(price) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все еще не обернуты по одному
No description provided.