Shalapugin Dmitry #35
Conversation
krepysh
left a comment
krepysh
left a comment
There was a problem hiding this comment.
Обрати внимание на то как ты называешь переменные и функции. Имена должны быть понятными и объяснять что в переменной лежит или что функция делает.
По списком лучше итерироваться сразу по объектем, а не по индексу по длине.
Не используй i,j,k, etc как имена переменных в циклах, кроме как для индексов.
for_challenges.py
Outdated
| names = ['Оля', 'Петя', 'Вася', 'Маша'] | ||
| # ??? | ||
| for i in is_male: | ||
| if is_male[i] == True: |
There was a problem hiding this comment.
c True никогда не нужно сравнивать. просто if something:
for_challenges.py
Outdated
| } | ||
| names = ['Оля', 'Петя', 'Вася', 'Маша'] | ||
| # ??? | ||
| for i in is_male: |
There was a problem hiding this comment.
Позапускай код, он кажется не делает то что нужно. Я бы ожидал здесь увидеть другой for.
for_challenges.py
Outdated
| # ??? | ||
| a = 0 | ||
| print(f'Всего {len(groups)} группы.') | ||
| for i in groups: |
for_challenges.py
Outdated
| # ??? | ||
| a = 0 | ||
| print(f'Всего {len(groups)} группы.') | ||
| for i in groups: |
There was a problem hiding this comment.
| for i in groups: | |
| for group in groups: |
for_challenges.py
Outdated
| a = 0 | ||
| print(f'Всего {len(groups)} группы.') | ||
| for i in groups: | ||
| a += 1 |
There was a problem hiding this comment.
В питоне при итерировании списка принято использовать сразу элементы списка, а не индексы. Не используй индексы без острой необходимости.
| return messages | ||
|
|
||
|
|
||
| def zadacha(messages): |
There was a problem hiding this comment.
Разбей код на несколько максимально независимых функций, вынеси print наружу.
Назови переменные понятными именами: ab, ac, ac, i, j, k - плохие названия для переменных, потому что не показывают что в них лежит и как будут использоваться.
string_challenges.py
Outdated
| # Вывести количество букв "а" в слове | ||
| word = 'Архангельск' | ||
| # ??? | ||
| print(len([j for j in ''.join(word) if j.lower() == 'а'])) |
There was a problem hiding this comment.
переусложнено - join тут вообще не нужен.
используй методы lower и count у str
string_challenges.py
Outdated
| # Вывести количество гласных букв в слове | ||
| word = 'Архангельск' | ||
| # ??? | ||
| print(len([j for j in ''.join(word) if j.lower() not in ['а', 'е', 'у', 'ы', 'о', 'э', 'я', 'и', 'ю']])) |
There was a problem hiding this comment.
вынеси список в перменную vowels
join не нужен
string_challenges.py
Outdated
| sentence = 'Мы приехали в гости' | ||
| # ??? | ||
| for i in ''.join(sentence).split(): | ||
| print(i[0]) |
There was a problem hiding this comment.
join не нужен
вместо i подошло бы имя переменной word
string_challenges.py
Outdated
| # Вывести усреднённую длину слова в предложении | ||
| sentence = 'Мы приехали в гости' | ||
| # ??? | ||
| q = [] |
krepysh
left a comment
krepysh
left a comment
There was a problem hiding this comment.
Much better!
Есть только замечание про поразбивать функции на функции попроще. Можешь потренироваться использовать в пайчарм рефакторинг:
Выдели курсором код который хочешь вынести в отдельную функцию, нажми правой кнопкой мыши - выбери Refactoring, используй Extract method функцию, Должна хорошо тут сработать.
for_challenges.py
Outdated
| names = ['Оля', 'Петя', 'Вася', 'Маша'] | ||
| # ??? | ||
| for name in names: | ||
| if name == 'Петя' or name == 'Вася': |
There was a problem hiding this comment.
здесь надо использовать is_male.
А что если мы хотим больше имен добавить в программу. Должно быть одно место в программе ответственное за вычисление пола по имени.
for_challenges.py
Outdated
| # ??? | ||
| counter_1 = 0 | ||
| for group in groups: | ||
| counter_1 += 1 |
There was a problem hiding this comment.
Используй enumerate, вместо того что бы считать индекс руками
for_challenges.py
Outdated
| ] | ||
| # ??? | ||
| count_groups = 0 | ||
| for group in groups: |
for_dict_challenges.py
Outdated
|
|
||
| for klass in school: | ||
| print( | ||
| f"Класс {klass['class']}: мальчики {[is_male[men_person['first_name']] for men_person in klass['students']].count(True)} " |
There was a problem hiding this comment.
Вынеси функцию расчета числа мальчиков и девочек в отдельную функцию, так что бы можно было ее переиспользовать в следующих задачах.
| return messages | ||
|
|
||
|
|
||
| def zadacha(messages): |
string_challenges.py
Outdated
| # Вывести усреднённую длину слова в предложении | ||
| sentence = 'Мы приехали в гости' | ||
| # ??? | ||
| spisok = [] |
There was a problem hiding this comment.
| spisok = [] | |
| words = [] |
krepysh
left a comment
krepysh
left a comment
There was a problem hiding this comment.
Почти отлично.
Поправь эту штуку, что бы у тебя klass передавался аргументом явно. Очень плохая практика лазить неявно к чему-то вне твоей локальной области видимости.
| ] | ||
| # ??? | ||
|
|
||
| for group in enumerate(groups, start=1): |
There was a problem hiding this comment.
распакуй же сразу:
| for group in enumerate(groups, start=1): | |
| for group_number, group in enumerate(groups, start=1): |
И в остальных местах тоже. Эта техника называется list unpacking, кода ты паре переменных присваиваешь значение листа.
|
|
||
| count = 0 | ||
| for klass in school_students: | ||
| most_common_name = Counter(name['first_name'] for name in school_students[count]).most_common(1) |
There was a problem hiding this comment.
school_students[count] это klass. Лучше использовать klass, и избавиться от count совсем.
| return [is_male[men_person['first_name']] for men_person in klass['students']].count(True) | ||
|
|
||
|
|
||
| def womens_count(): |
There was a problem hiding this comment.
klass должен передаваться явно как аргумент (можно еще и назвать его по другому, что бы точно не было сомнений.
2 participants
No description provided.