-
Notifications
You must be signed in to change notification settings - Fork 260
for_challenges и for_dict_challenges на проверку #42
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
for_challenges.py
Outdated
| if is_male[name]: | ||
| print(f'{name}: Мужской') | ||
| else: | ||
| print(f'{name}: Женский') |
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.
👍
Можно для тренировки подумать а что если is_male вдруг не содержит такого имени?
for_challenges.py
Outdated
| print('\nЗадание 4') | ||
| print(f'Всего {len(groups)} группы') | ||
| for group in range(len(groups)): | ||
| print(f'Группа {group + 1}: {len(groups[group])} ученика') |
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.
👍
Можно почитать про enumerate. Будет чуть поудобнее, Так тоже хорошо)
for_challenges.py
Outdated
| ] | ||
| # ??? | ||
| print('\nЗадание 5') | ||
| [print(' '.join(group)) for group in groups] |
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.
Если честно, не совсем понял задание. Показалось, что в описании задачи опечатка в слове "пары" (я заменил на "группы"). Потому, что если судить по примеру вывода, то с новой строки нужно вывести каждую группу поименно. Пока что подправил код ближе к примеру. Если задание нужно сделать по-другому, прошу уточнить) 4f42a44
| qty_students = (Counter(student['first_name'] for student in students)) | ||
| for student in qty_students: | ||
| print(f'{student}: {qty_students[student]}') | ||
| print() |
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.
Fair enough.
работать будет. А можешь любое из этих заданий сделать напрямую через словари, без counter?
| most_common_student = Counter(student['first_name'] | ||
| for student in students).most_common(1) | ||
| print( | ||
| f'Самое частое имя среди учеников: {most_common_student[0][0]}\n') |
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 student in school_students[grade]).most_common(1) | ||
| print( | ||
| f'Самое частое имя в классе {grade + 1}: {max_students[0][0]}') | ||
| print() |
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.
👍
| female += 1 | ||
| print( | ||
| f"Класс {grade['class']}: девочки {female}, мальчики {male}") | ||
| print() |
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
| else: | ||
| female[grade] += 1 | ||
|
|
||
| most_male = school[male.index(max(male))]['class'] |
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: | ||
| if is_male[name]: | ||
| print(f'{name}: Мужской') | ||
| else: | ||
| print(f'{name}: Женский') | ||
| except KeyError: | ||
| print(f'Не знаю такого имени: {name}') |
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.
#42 (comment)
Добавил try...except
for_challenges.py
Outdated
| for group in range(len(groups)): | ||
| print(f'Группа {group + 1}: {len(groups[group])} ученика') | ||
| for index, group in enumerate(groups, start=1): | ||
| print(f"Группа {index}: {len(group)} ученика") |
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.
#42 (comment)
Сделал с enumerate, выглядит приличнее)
for_dict_challenges.py
Outdated
| # qty_students = (Counter(student['first_name'] for student in students)) | ||
| # for student in qty_students: | ||
| # print(f'{student}: {qty_students[student]}') | ||
| # print() | ||
| unique_students = [] | ||
| for student in students: | ||
| if student['first_name'] not in unique_students: | ||
| unique_students.append(student['first_name']) | ||
| for student in unique_students: | ||
| print(f"{student}: {students.count({'first_name': student})}") |
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.
#42 (comment)
Попробовал без Counter
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.
А зачем тебе отдельный unique_students, если ты можешь проверять напрямую в словаре? проверка находится ли ключ в словаре работает ощутимо быстрее, чем проверка есть ли значение в словаре
for_dict_challenges.py
Outdated
| most_male = school[male.index(max(male))]['class'] | ||
| most_female = school[female.index(max(female))]['class'] | ||
| most_male_index = male.index(max(male)) | ||
| most_male_class = school[most_male_index]['class'] | ||
| most_female_index = female.index(max(female)) | ||
| most_female_class = school[most_female_index]['class'] | ||
| print( | ||
| f"Больше всего мальчиков в классе {most_male}") | ||
| f"Больше всего мальчиков в классе {most_male_class}") | ||
| print( | ||
| f"Больше всего девочек в классе {most_female}") | ||
| f"Больше всего девочек в классе {most_female_class}") |
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.
#42 (comment)
Решил располовинить на индекс класса и класс)
|
Тут стоит поправить только часть про counter так чтоб избавиться от лишнего массива |
|
|
||
|
|
||
| def count_user_messages(message): | ||
| if users_messages_counter.get(message['sent_by']): |
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.
Такая конструкция в данном случае сработает хорошо, но в других ситуациях может сыграть злую шутку.
так как провалится в else в случае если users_messages_counter[message['sent_by']] интерпретируется как False, то есть и None, и 0 и пустая строка и false.
Более корректно использовать проверку
if users_messages_counter.get(message['sent_by']) is not None:
| if users_messages_counter.get(message['sent_by']): | ||
| users_messages_counter[message['sent_by']] += 1 | ||
| else: | ||
| users_messages_counter[message['sent_by']] = 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.
Да, так хорошо. Далее для упрощения можно пользоваться defaultdict
| if message.get('reply_for'): | ||
| quoted_user_id = user_by_message[message['reply_for']] | ||
| user_replied_amount[quoted_user_id] = user_replied_amount.get( | ||
| quoted_user_id, 0) + 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.
Отлично. Изящно получилось
| return users_seen_by | ||
|
|
||
|
|
||
| def count_max_views(seen_by_dict): |
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 get_max_replied_messages(replied_messages, amount=5): | ||
| sorted_messages = sorted( | ||
| replied_messages.items(), key=lambda item: len(item[1]), reverse=True) |
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('Most messages are in the: ', get_most_messaged_time_period( | ||
| messages_by_time)) # Задание 4 | ||
| pprint(get_max_replied_messages( | ||
| most_replied_messages, 3)) # Задание 5 |
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.
В целом все круто и технично. Над чем важно поработать: чистые и грязные функции. Сейчас у тебя почти все функции грязные так как пользуются глобальными переменными. Уже на таком объеме, читая с нуля становится сложно контролировать какой словарь в какой момент был создан и когда менялся.
Более чистое решение выглядело бы так
messages = generate_chat_history()
users_seen_by = count_message_views(messages)
или так
users_messages_counter, user_by_message, ..., most_replied_messages = prepare_messages_data(messages)
Второй метод намного хуже модифицируется, поэтому я бы не боялась несколько раз пройтись по messages
отдельно для каждого задания и получить для каждого задания свой результат.
Такой подход позволит дальше тебе модифицировать код без больших издержек
No description provided.