Badriev A. (Basic_exercises) by badrievad · Pull Request #36 · learnpythonru/basic_exercises

badrievad · 2023-05-19T05:04:59Z

No description provided.

krepysh

В целом здорово: несколько замечаний ко всему коду:

Не используй lambda функции без острой необходимости (в этих задачах ее нет, избавься от всех лямбд)
Лучше разделяй код: return вместо print, выноси инпут наружу, разделяй сложные функции на функции попроще. Где возможно переиспользуй фунции: использую решение предыдущей задачи как подпрограмму.
Наименование функции должно отражать что она возвращает.

krepysh · 2023-05-19T11:56:33Z

for_challenges.py

 ]
-# ???

+group_token = lambda group_to: (


Не используй лямбда функции без острой необходимости
К тому же здесь ты даже ей имя даешь. Обычный def будет читаемее

https://towardsdatascience.com/the-python-lambda-function-has-become-a-devil-d0340404abcb

Переделай в обычные функции

krepysh · 2023-05-19T12:00:22Z

for_challenges.py

+
+student_token = lambda student_to: (
+        (student_to in range(5, 20)) and 'учеников' or
+        (1 in (student_to, (diglast := student_to % 10))) and 'ученик' or


Классно что было интересно заняться этим, но это не подразумевалось в задании.
Доп. вопрос можно ли сделать это решение универсальнее, что бы не можно было использовать эту логику для почти любого существительного : "парта" "шкаф"?

krepysh · 2023-05-19T12:01:44Z

for_challenges.py

 ]
-# ???
+
+for key, value in dict(enumerate(groups, start=1)).items():


вот тут не нужен dict().items()
просто for group, group_number in enumerate.
и постарайся запомнить порядок элементов в tuple от enumerate.

krepysh · 2023-05-19T12:02:15Z

for_dict_challenges.py

-# ???
+names = []
+for student in students:
+    names.append(student['first_name'])


Можно ли переделать с использование list comprehension?

krepysh · 2023-05-19T12:03:09Z

for_dict_challenges.py

+print('_' * 75)

+# print(pd.DataFrame(names).mode()[0][0])
+print(f'Самое частое имя среди учеников: {max(names, key=lambda x: names.count(x))}')


Посчитай по честному, через числа в словаре

krepysh · 2023-05-19T12:15:05Z

for_dict_challenges_bonus.py

+    }
+
+    for message in messages:
+        if float(message["sent_at"].strftime('%H.%M')) < 12:


Это очень непрямой способ проверять время.
Судя по всему sent_at это datetime, просто обратись к атрибуту .hour
и вытащи это в отдельную переменную, что бы message["sent_at"] использовался только один раз в этом цикле, а в if проверяй эту переменную.

krepysh · 2023-05-19T12:16:59Z

for_dict_challenges_bonus.py

+def task5(messages):
+    result = []
+
+    for k, g in groupby([message['reply_for'] for message in messages]):


k, g - плохие имена
Назови так что бы было понятно что там лежит.

krepysh · 2023-05-19T12:17:27Z

for_dict_challenges_bonus.py

+        result.append((k, length))
+
+    lst = [(key, value) for key, value in sorted(result, key=lambda x: x[1], reverse=True) if
+           key is not None and value > 1]


А зачем этот if?

krepysh · 2023-05-19T12:18:02Z

for_dict_challenges_bonus.py

+
+    if len(lst) > 0:
+        for key, value in lst:
+            print(f'Сообщение под айди: "{key}" стало началом для самых длинных тредов (цепочек ответов), а именно '


return instead of print

krepysh · 2023-05-19T12:18:31Z

string_challenges.py

 word = 'Архангельск'
-# ???
-
+vowels = 'аеиоуэюяыё'


krepysh

Значительно лучше, пара мест в которых можно поправить нейминг, и одно место в котором создаешь слишком много объектов set, а это относительно дорого по времени (и потенциально памяти)

krepysh · 2023-05-23T16:02:39Z

for_challenges.py

+        return 'групп'
+    elif 1 in (numb, (diglast := numb % 10)):
+        return 'группа'
+    elif {numb, diglast} & {2, 3, 4}:


Тот редкий случай когда строчка комментария не помешала бы.

krepysh · 2023-05-23T16:04:58Z

for_dict_challenges.py

 # Петя: 2

+# создадим функцию, которая возвращает список студентов по ключу 'first_name'
+def student_list(lst):


нэйминг надо поправить. пример: get_students_first_names(student_list):

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

krepysh · 2023-05-23T16:12:46Z

for_dict_challenges.py

-# ???


+def find_girls():


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

И передавай group явно, через аргументы.
Обращаться к какому-то объекту из внешней области видимости плохая практика. Это приводит к неочевидным ошибкам. В твоем случае функции перестанут работать если ты переименуешь переменную в цикле.

krepysh · 2023-05-23T16:13:42Z

for_dict_challenges.py

-
+for group in school:
+    if find_boys() > find_girls():
+        print(f'Больше всего мальчиков в классе: {group["class"]}')


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

krepysh · 2023-05-23T16:14:22Z

for_dict_challenges_bonus.py

+    return maximum_frequency(create_list_by_key('sent_by', messages))
+
+
+def find_id_who_got_the_most_replies(messages):


Классный нейминг!

krepysh · 2023-05-23T16:17:10Z

for_dict_challenges_bonus.py

+        if message['sent_by'] not in id_unique:
+            id_unique[message['sent_by']] = set(message['seen_by'])
+        else:
+            id_unique[message['sent_by']] = set(id_unique[message['sent_by']]) | set(message['seen_by'])


Это кажется дорогая операция, много раз пересоздаешь сет.

Suggested change

id_unique[message['sent_by']] = set(id_unique[message['sent_by']]) | set(message['seen_by'])

id_unique[message['sent_by']] = id_unique[message['sent_by']].update(message['seen_by'])

Если использовать id_unique.get(message['sent_by'], set()), то можно еще и от if избавиться.

krepysh · 2023-05-23T16:20:59Z

for_dict_challenges_bonus.py

+    for message in messages:
+        hour_of_writing = message["sent_at"].hour
+        if hour_of_writing < 12:
+            times['morning'].append(hour_of_writing)


с таким же успехом тут может быть переменная список morning. Не вижу как словарь тут упрощает что-то.

badrievad added 7 commits May 13, 2023 15:47

Test commit

7eabf14

for_challenges done

f3d43f8

string_challenges done

a3d77c9

for_dict_challenges done

19e2ef3

for_dict_challenges_bonus done

eb457db

Done

340e25e

Done

24a75a7

krepysh suggested changes May 19, 2023

View reviewed changes

badrievad and others added 2 commits May 20, 2023 21:24

Done

29014c5

Update for_dict_challenges.py

5e8c03b

krepysh suggested changes May 23, 2023

View reviewed changes

badrievad added 5 commits May 24, 2023 14:36

Done

ec1b21b

Done

c115653

Done

2d839db

Done

84f10c7

Done

3fcdf93

		return maximum_frequency(create_list_by_key('sent_by', messages))


		def find_id_who_got_the_most_replies(messages):

	id_unique[message['sent_by']] = set(id_unique[message['sent_by']]) \| set(message['seen_by'])
	id_unique[message['sent_by']] = id_unique[message['sent_by']].update(message['seen_by'])

Conversation

badrievad commented May 19, 2023

Uh oh!

krepysh left a comment

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!

krepysh left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants