Форум программистов, компьютерный форум, киберфорум
Python
Войти
Регистрация
Восстановить пароль
Карта форума Темы раздела Блоги Сообщество Поиск Заказать работу  
 
Рейтинг 4.75/4: Рейтинг темы: голосов - 4, средняя оценка - 4.75
0 / 0 / 0
Регистрация: 12.04.2018
Сообщений: 7
1

Требуется критика кода

12.04.2018, 17:54. Показов 707. Ответов 13
Метки нет (Все метки)

Author24 — интернет-сервис помощи студентам
Доброго времени суток, уважаемые форумачане. Хотел бы вас попросить покритиковать код. Версия python 3.6. Код во вложении.
В заранее благодарен.

Python
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
import random
 
 
def search_position_letter(l, w):
    letter_position = w.index(l)
    return letter_position
 
def string_word_to_list_word(w):
    '''str(obj) => list(obj)'''
    list_word = list(w)
    return list_word
 
def list_word_to_string_word(w):
    '''list(obj) => str(obj)'''
    string_word = ''.join(w)
    return string_word
 
def same_letter(user_answer, word):
    '''Находит и возврощает список с индексами повторяющихся символов'''
    letter_list = []
    word = list(word)
    while user_answer in word:
        letter_list.append(word.index(user_answer))
        number = len(letter_list)
        letter_list[-1] += number - 1
        del word[word.index(user_answer)]
    return letter_list
 
def user_attempts_choice(user_attempts):
    if user_attempts == 1:
        attempts = 7
        return attempts
    elif user_attempts == 2:
        attempts = 5
        return attempts
    elif user_attempts == 3:
        attempts = 3
        return attempts
    else:
        return 'Неверный ввод.' 
 
 
words_list = ['книга', 'месяц', 'ручка', 'шарик', 'олень', 'носок']
 
while len(words_list) != 0:
    word = random.choice(words_list)
    crypto_word = '\u25A0' * len(word)
    user_attempts = int(input('Выберите уровень сложности: 1-легкий; 2-средний; 3-сложный: '))
    attempts = user_attempts_choice(user_attempts)
    print(f'В слове {len(word)} букв.')
    print(f'У вас есть {attempts} попыток, чтобы угадать слово.')
    while '\u25A0' in crypto_word:
        if attempts == 0:
            print('К сожалению у вас закончились попытки!')
            break
        user_answer = input('Введите букву или слово целиком: ')
        if user_answer == word:
            break
        if user_answer in word:
            crypto_word = string_word_to_list_word(crypto_word)
            if word.count(user_answer) > 1:
                letter_list = same_letter(user_answer, word)
                for i in letter_list:
                    crypto_word[i] = user_answer
            letter_position = search_position_letter(user_answer, word)
            crypto_word[letter_position] = user_answer
            crypto_word = list_word_to_string_word(crypto_word)
            print(crypto_word)
        else:
            print('Нет такой буквы!')
            attempts -= 1
            print(f'Осталось попыток: {attempts}')
    print(f'Поздравляем, это и правда слово "{word}"')
    del words_list[words_list.index(word)]
print('Поздравляем, вы угадали все доступные слова.')
Вложения
Тип файла: zip fortuna_demo.py.zip (1.1 Кб, 1 просмотров)
0
Programming
Эксперт
94731 / 64177 / 26122
Регистрация: 12.04.2006
Сообщений: 116,782
12.04.2018, 17:54
Ответы с готовыми решениями:

Требуется оценка и критика моего кода готового приложения
Написал программу https://github.com/XanderWhite/LEARCA (программа для изучения карточек, н-р...

Требуется конструктивная критика
Я не волшебник, только учусь. Для обучения выбрал алгоритм поиска пути A*. Вот собственно моя...

Требуется критика по коду
Приветствую! Я недавно начал заниматься фрилансом, пока что были заказы по HTML + CSS, JS, но...

UI controller. требуется критика
Пишу приложение, состоящее из UI объектов. UI объекты есть как статические, добавленные в...

Оцените код. Требуется критика
Добрый день, я начал изучать js,nodejs и jq. Делаю сейчас простые проекты, на которых можно...

13
Эксперт Python
4632 / 2050 / 361
Регистрация: 17.03.2012
Сообщений: 10,134
Записей в блоге: 6
12.04.2018, 18:13 2
Цитата Сообщение от lexcher Посмотреть сообщение
del word[word.index(user_answer)]
Не знаю, какой у вас там размер списка, но лучше так не искать, использовать словарь или множество.
0
0 / 0 / 0
Регистрация: 12.04.2018
Сообщений: 7
12.04.2018, 18:23  [ТС] 3
Цитата Сообщение от dondublon Посмотреть сообщение
Не знаю, какой у вас там размер списка, но лучше так не искать, использовать словарь или множество.
В данной строке, я нахожу индекс слова в списке исходя из правильного ответа пользователя, затем удаляю его из списка, чтобы продолжить игру, и так до тех пор пока список не станет пустым. По заданию было указанно, что требуется создать именно список со словами. Пока что еще не дотумкал каким образом еще можно исключить из списка уже 'отыгранное' слово.
0
672 / 475 / 215
Регистрация: 06.09.2013
Сообщений: 1,306
12.04.2018, 18:29 4
lexcher, первые две функции лишние, на мой взгляд
вторые две можно упростить как нибудь так:
Python
1
2
3
4
5
6
def same_letter(user_answer, word):
    '''Находит и возврощает список с индексами повторяющихся символов'''
    return [idx for idx, ch in enumerate(word) if user_answer == ch]
 
def user_attempts_choice(user_attempts):
    return {1:7, 2:5, 3:3}.get(user_attempts, 'Неверный ввод')
Вот этот блок можно убрать и рассматривать много угаданных букв как частный случай одной:
Python
1
2
letter_position = search_position_letter(user_answer, word)
crypto_word[letter_position] = user_answer
Зачем каждый раз перегонять crypto_word из списка в строку, храните всегда в списке и только на экран выводите через str.join.

Да и вообще как-то не очень, условных операторов много - зачем, если есть словари и полиморфизм, щас так не пишут, вроде.
Хотя я питон недавно изучать начал, но это то что бросается в глаза.
0
0 / 0 / 0
Регистрация: 12.04.2018
Сообщений: 7
12.04.2018, 18:37  [ТС] 5
Цитата Сообщение от woldemas Посмотреть сообщение
Вот этот блок можно убрать и рассматривать много угаданных букв как частный случай одной:

letter_position = search_position_letter(user_answer, word)
crypto_word[letter_position] = user_answer
Не совсем понял.
0
672 / 475 / 215
Регистрация: 06.09.2013
Сообщений: 1,306
12.04.2018, 18:52 6
Цитата Сообщение от lexcher Посмотреть сообщение
Не совсем понял.
Ну вот я чуть подчистил, что мне в глаза бросается, но я бы вообще по-другому написал
Кликните здесь для просмотра всего текста

Python
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
import random
   
def same_letter(user_answer, word):
    '''Находит и возврощает список с индексами повторяющихся символов'''
    return [idx for idx, ch in enumerate(word) if user_answer == ch]
 
def user_attempts_choice(user_attempts):
    return {1:7, 2:5, 3:3}.get(user_attempts, 'Неверный ввод')
 
words_list = ['книга', 'месяц', 'ручка', 'шарик', 'олень', 'носок']
 
while len(words_list) != 0:    
    word = random.choice(words_list)    
    crypto_word = ['\u25A0'] * len(word)     
    user_attempts = int(input('Выберите уровень сложности: 1-легкий; 2-средний; 3-сложный: '))
    attempts = user_attempts_choice(user_attempts)
    print(f'В слове {len(word)} букв.')
    print(f'У вас есть {attempts} попыток, чтобы угадать слово.')
    while '\u25A0' in crypto_word:
        if attempts == 0:
            print('К сожалению у вас закончились попытки!')
            break
        user_answer = input('Введите букву или слово целиком: ')
        if user_answer == word:
            break                         
        letter_list = same_letter(user_answer, word)
        if len(letter_list):            
            for i in letter_list:
                crypto_word[i] = user_answer            
            print(''.join(crypto_word))
        else:
            print('Нет такой буквы!')
            attempts -= 1
            print(f'Осталось попыток: {attempts}')
    print(f'Поздравляем, это и правда слово "{word}"')
    words_list.remove(word)
print('Поздравляем, вы угадали все доступные слова.')
1
0 / 0 / 0
Регистрация: 12.04.2018
Сообщений: 7
12.04.2018, 18:54  [ТС] 7
Цитата Сообщение от woldemas Посмотреть сообщение
Ну вот я чуть подчистил, что мне в глаза бросается, но я бы вообще по-другому написал
Благодарю, сейчас ознакомлюсь. Сразу оговорюсь с ООП еще не знаком, на данный момент писал код таким каким он мне казался очевидным))
0
672 / 475 / 215
Регистрация: 06.09.2013
Сообщений: 1,306
12.04.2018, 18:55 8
Цитата Сообщение от lexcher Посмотреть сообщение
Сразу оговорюсь с ООП еще не знаком
Там нет ООП, я ваш код мало поменял
0
0 / 0 / 0
Регистрация: 12.04.2018
Сообщений: 7
12.04.2018, 19:00  [ТС] 9
Цитата Сообщение от woldemas Посмотреть сообщение
Там нет ООП, я ваш код мало поменял
Да, да я понял. Последнюю тему которую изучал были Функции, и как я понимаю я создал лишние функции. Пытался уделять им больше внимания, в то время как они только нагромождали код.
0
672 / 475 / 215
Регистрация: 06.09.2013
Сообщений: 1,306
12.04.2018, 19:19 10
Цитата Сообщение от lexcher Посмотреть сообщение
как я понимаю я создал лишние функции
Что касается функций,например, в данном случае можно выделить в функцию один раунд игры, например таким образом, тогда код читабельнее будет (еще я там выход добавил):
Кликните здесь для просмотра всего текста

Python
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
def begin_round(word, attempts):
    crypto_word = ['\u25A0'] * len(word)     
    while '\u25A0' in crypto_word: 
        if attempts == 0:
            print('К сожалению у вас закончились попытки!')
            return False            
        user_answer = input('Введите букву или слово целиком: ')
        if user_answer == word: 
            return True            
        letter_list = same_letter(user_answer, word)
        if len(letter_list):            
            for i in letter_list: crypto_word[i] = user_answer            
            print(''.join(crypto_word))
        else:
            print('Нет такой буквы!')
            attempts -= 1
            print(f'Осталось попыток: {attempts}')
    return True
 
words_list = ['книга', 'месяц', 'ручка', 'шарик', 'олень', 'носок']
while len(words_list) != 0:    
    word = random.choice(words_list)         
    attempts = int(input('Выберите уровень сложности: 1-легкий; 2-средний; 3-сложный: '))
    if begin_round(word, user_attempts_choice(attempts)):
        print(f'Поздравляем, это и правда слово "{word}"')
    else: 
        print(f'Вы не угадали слово "{word}"')
    words_list.remove(word)
    if input('Продолжить: д/н?').lower() != 'д': 
        break
0
0 / 0 / 0
Регистрация: 12.04.2018
Сообщений: 7
12.04.2018, 19:24  [ТС] 11
Цитата Сообщение от woldemas Посмотреть сообщение
def begin_round(word, attempts):
* * crypto_word = ['\u25A0'] * len(word) * *
* * while '\u25A0' in crypto_word:
* * * * if attempts == 0:
* * * * * * print('К сожалению у вас закончились попытки!')
* * * * * * return False * * * * * *
* * * * user_answer = input('Введите букву или слово целиком: ')
* * * * if user_answer == word:
* * * * * * return True * * * * * *
* * * * letter_list = same_letter(user_answer, word)
* * * * if len(letter_list): * * * * * *
* * * * * * for i in letter_list: crypto_word[i] = user_answer * * * * * *
* * * * * * print(''.join(crypto_word))
* * * * else:
* * * * * * print('Нет такой буквы!')
* * * * * * attempts -= 1
* * * * * * print(f'Осталось попыток: {attempts}')
* * return True

Это нормальная практика, чтобы функция что то печатала через print, а не только возвращала посредством return?
0
672 / 475 / 215
Регистрация: 06.09.2013
Сообщений: 1,306
12.04.2018, 19:29 12
Цитата Сообщение от lexcher Посмотреть сообщение
Это нормальная практика, чтобы функция что то печатала через print
не знаю, но для данной задачи - пойдет
Можно туда в функцию еще и вот это вывод засунуть, логичнее будет:
Python
1
2
3
        print(f'Поздравляем, это и правда слово "{word}"')
    else: 
        print(f'Вы не угадали слово "{word}"'
0
Эксперт Python
4632 / 2050 / 361
Регистрация: 17.03.2012
Сообщений: 10,134
Записей в блоге: 6
13.04.2018, 11:52 13
Цитата Сообщение от lexcher Посмотреть сообщение
В данной строке, я нахожу индекс слова в списке исходя из правильного ответа пользователя, затем удаляю его из списка, чтобы продолжить игру,
Я говорю не о ваших целях, а о структуре данных, которую вы для этого используете. Вы используете список, надо словарь или множество.
0
0 / 0 / 0
Регистрация: 12.04.2018
Сообщений: 7
13.04.2018, 14:13  [ТС] 14
Цитата Сообщение от dondublon Посмотреть сообщение
Я говорю не о ваших целях, а о структуре данных, которую вы для этого используете. Вы используете список, надо словарь или множество.
Цитата Сообщение от lexcher Посмотреть сообщение
По заданию было указанно, что требуется создать именно список со словами.
Но все равно благодарю, за совет.
0
13.04.2018, 14:13
IT_Exp
Эксперт
87844 / 49110 / 22898
Регистрация: 17.06.2006
Сообщений: 92,604
13.04.2018, 14:13
Помогаю со студенческими работами здесь

Критика кода на F#
Прошу прошения, что создаю тему без конкретного вопроса, просто знакомство с F# дается с трудом, и...

Критика кода
Здравствуйте форумчане, я сейчас в процессе изучения создания сайтов (сейчас учу js/php), понемногу...

Шахматы - критика кода
Думать пока не могут, только доска с возможностью делать ходы за обе стороны. Все правила игры...

Массивы, критика кода
Дорогие друзья, такой вопрос что не так в моем коде?#include <iostream> #include <stdlib.h> ...

Нужна критика кода
Хочу услышать замечания и предложения по коду, может у кого-то есть идеи как в данном коде можно...

Калькулятор - критика кода
Сплошной примитив и способ реализации уверен не самый удачный, но я новичок в Си и для меня это...


Искать еще темы с ответами

Или воспользуйтесь поиском по форуму:
14
Ответ Создать тему
КиберФорум - форум программистов, компьютерный форум, программирование
Powered by vBulletin
Copyright ©2000 - 2024, CyberForum.ru