-
Notifications
You must be signed in to change notification settings - Fork 260
Remade and schortened #43
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
base: main
Are you sure you want to change the base?
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.
В коменте к пул-реквесту написано, что поправил пятое задание из бонусных, а в пул-реквесте вообще все задания. Непонятно что смотреть.
Я погляжу всё, но старайся пжл чтобы в пул-реквестах не было того, что смотреть не надо.
for_dict_challenges.py
Outdated
| for inf_student in students: | ||
| student_name = inf_student['first_name'] | ||
| if student_name in number_repetitions_names: | ||
| number_repetitions_names[student_name] = number_repetitions_names.get(student_name) + 1 |
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.
Тут не нужно использовать get тк в условии ифа ты проверил, что такой ключ точно есть
for_dict_challenges.py
Outdated
| class_students = {} | ||
| for every_schoolkid in every_class: | ||
| if every_schoolkid['first_name'] in class_students: | ||
| count_name += 1 |
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.
В этой переменной какая-то беда: ты вроде считаешь повторение одного имени, но переменная общая на все имена: обнуляется только перед первым циклом.
Я бы советовал вообще избавиться от этой переменной.
for_dict_challenges_bonus.py
Outdated
| return one_message['sent_by'] | ||
|
|
||
|
|
||
| # Решение задания №3 |
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.
Лишний коммент
for_dict_challenges_bonus.py
Outdated
|
|
||
|
|
||
| def message_with_max_viewing(messages): | ||
| users_with_users = create_users_with_unique_users(messages) |
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.
По названию непонятно, что в этой переменной. По названию функции тоже непонятно, что она делает.
"Создать пользователей с уникальными пользователями"?..
for_dict_challenges_bonus.py
Outdated
|
|
||
| def get_id_last_messages_in_thread(id_with_reply_for): | ||
| id_message_for_reply = [id for id in id_with_reply_for | ||
| if id_with_reply_for.get(id) is 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.
Советую форматировать многострочные лист компрехеншены от так:
id_message_for_reply = [
id for id in id_with_reply_for
if id_with_reply_for.get(id) is 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.
А ещё тут условие можно упростить до if id in id_with_reply_for
А ещё id – это встроенная в питон функция, так лучше не называть переменные.
|
При создании пул-реквеста есть выбиралка, в какой репозиторий делать пул-реквест. Ты сделал пул-реквест в основной репо learnpythonru, так лучше не делать. Делай пр в свой репо. |
Исправил замечания. Переделал и сократил решение Задания №5