Asenim/HelloWorld140 - Hangman.md

## HelloWorld140 - Hangman.md

      
    Raw
  

              HelloWorld140 - Hangman.md
            
          
    https://github.com/HelloWorld140/Hangman

Ревью от Алексея @Raketa4000az
[Hello World]
Простая реализация в процедурном стиле. Выполнено в двух классах.
Недостатки реализации:

Странная нумерация уровней: от 2 до 4. Почему нумерация не от 1?

Выберите уровень сложности введя цифру от 2 до 4: 
Легко (2)
Средне (3)
Сложно (4)

На старте игры выводится отгадываемое слово

Выберите уровень сложности введя цифру от 2 до 4: 
Легко (2)
Средне (3)
Сложно (4)
4
[п, о, д, н, о, с, ч, и, ц, а]

При выигрыше нужно показать слово целиком

д о й н и □ 
Ранее введеные буквы: [ф, д, о, й, н, и]
Количество ошибок: 1 из 8
Введите 1 русскую букву
к
Вы угадали букву!
Поздравлю, вы победили!

Для начала игры введите (д).
Хорошо:

Игра запускается
3 уровня сложности
Есть список введенных букв
Можно ввести только одиночную русскую букву
Широкое применение констант
Короткие методы
Простой понятный алгоритм

Замечания:
0. Мусор в гит репозитории: .idea и .iml не надо комитить, используй гитигнор

Нейминг
-Из название не ясно, для чего используется эта переменная. "Буква как char"- что это значит? И почему оно хранит пробел?
private static char letterAsChar = ' ';

-Название вводит в заблуждение. В енаме хранятся не ошибки, а картинки хангмана
enum Mistakes
--
Oracle Java code conventions, part."Naming conventions"
Мартин, "Чистый код", гл.2
-- 

Нарушение конвенции кода, константы под полями класса

private static boolean playerChoice;
private static final int EASY = 2;

Нарушение конвенции кода, в любой ситуации выделяй блоки

else if (!Character.isDigit(letter.charAt(0)))
  System.out.println("Вы ввели не цифру. Введите цифру 1, 2 или 3.");
else if (Integer.parseInt(letter) > HARD || Integer.parseInt(letter) < EASY)
  System.out.println("Вы ввели неверную цифру. Введите цифру 1, 2 или 3.");

//ПРАВИЛЬНО:
else if (!Character.isDigit(letter.charAt(0))) {
  System.out.println("Вы ввели не цифру. Введите цифру 1, 2 или 3.");
} else if (Integer.parseInt(letter) > HARD || Integer.parseInt(letter) < EASY) {
  System.out.println("Вы ввели неверную цифру. Введите цифру 1, 2 или 3.");
}


Нарушение инкапсуляции, все методы публичные. В class Hangman публичным должен быть только void main(String[] args).


Нарушение DRY, магические буквы, числа, слова. Вводи константы. А если они уже есть- используй


System.out.println("Выберите уровень сложности введя цифру от 2 до 4: \nЛегко (2)\nСредне (3)\nСложно (4)");
//...
System.out.println("Вы вввели больше 1 цифры. Введите цифру 2, 3 или 4.");

//ПРАВИЛЬНО:
System.out.printf("Выберите уровень сложности введя цифру: \nЛегко (%d)\nСредне (%d)\nСложно (%d) \n", EASY, MEDIUM, HARD);
//...
System.out.printf("Вы вввели больше 1 цифры. Введите цифру %d, %d или %d. \n", EASY, MEDIUM, HARD);
--
Фаулер, "Рефакторинг", гл.8 п."Замена магического числа символической константой"
refactoring.guru "Замена магического числа символьной константой"
--

Создавай вспомогательные методы, делай программу более простой и понятной

while (counterOfMistakes != Math.ceil((double) MAX_COUNT_OF_MISTAKES / difficulty) && !HIDDEN_WORD.equals(WORD)) {...}

//ПРАВИЛЬНО:
while (isНазваниеКотороеВсеОбъясняет(/*args or empty*/)) {...}

private boolean isНазваниеКотороеВсеОбъясняет(/*args or empty*/) {
  return counterOfMistakes != Math.ceil((double) MAX_COUNT_OF_MISTAKES / difficulty) && !HIDDEN_WORD.equals(WORD);
}

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

System.out.println("\nДля начала игры введите (" + YES + ").\nДля закрытия введите (" + NO + ")");

//ПРАВИЛЬНО:
System.out.printf("\nДля начала игры введите (%s). \nДля закрытия введите (%s) \n", YES, NO);

Если в блоке if есть return(break, continue, throw, exit и т.д.), то else не пишется - это не имеет смысла

else if (Integer.parseInt(letter) == EASY) {
  difficulty = EASY;
  break;
} else if (Integer.parseInt(letter) == MEDIUM) {
  difficulty = MEDIUM;
  break;
} else if (Integer.parseInt(letter) == HARD) {
  difficulty = HARD;
  break;
}

//ПРАВИЛЬНО:
else if (Integer.parseInt(letter) == EASY) {
  difficulty = EASY;
  break;
} 
if (Integer.parseInt(letter) == MEDIUM) {
  difficulty = MEDIUM;
  break;
} 
if (Integer.parseInt(letter) == HARD) {
  difficulty = HARD;
  break;
}

Побочные эффекты, метод делает несколько дел одновременно.
Один метод должен делать только одно дело(одну ответственность, SRP). В данном случае метод должен только проверять букву.
Но кроме этого метод выполняет побочное действие- изменяет поле класса

public static boolean checkInputLetterInWord(char letter) {
  //...  
  HIDDEN_WORD.set(i, letter);
  //...
  return HIDDEN_WORD.contains(letter);
}
Метод нужно разделить на два: один должен проверять букву на корректность, второй- устанавливать букву в слове.
И тогда вместо
if (!checkInputLetterInWord(letterAsChar)) {
  System.out.println("Вы не угадали букву!");
  counterOfMistakes++;
} else System.out.println("Вы угадали букву!");

Будет
if(isЕстьТакаяБуква(letter)) {
  открытьБукву(letter);
  System.out.println("Вы угадали букву!");
} else {
  System.out.println("Вы не угадали букву!");
  counterOfMistakes++;
}
То же самое относится к boolean checkAlreadyInputLetter(char letter).
---
Мартин, "ЧК", гл.3,п."Избавьтесь от побочных эффектов"
---

Вот это крайне загадочная тема
if (counterOfMistakes == Math.ceil((double) MAX_COUNT_OF_MISTAKES / difficulty)) {...}
Я так понимаю, что Math.ceil(...) высчитывает максимальное количество  ошибок на текущем уровне сложности.
Тогда не высчитывай этот параметр каждый раз, а установи его один раз там, где устанавливается уровень сложности:

public static void mainGameLoop() {
  while (startOrEndGame()) {
    choiceDifficultyInput();
    установитьМаксимальноеКоличествоОшибокНаТекущемУровнеСложности(); 
    //...
  }
}

//...
if (counterOfMistakes == maxLevelMistakes) {...}

Распечатка картинки висельницы черес switch-case или if-elseif - индусский код.
Картинки нужно хранить в статическом массиве и печатать по номеру, например, так

private static final String[][] PICTURES = {
  {
    "-----   ",
    "|       ",
    "|       ",
    "|       ",
    "|       ",
    "|       ",
    "------- ",
   },
   {
     "-----   ",
     "|   |   ",
     "|   O   ",
     "|       ",
     "|       ",
     "|       ",
     "------- ",
   },
   // more pics
  };

void printPicture(int numPicture) {  
  String[] picture = PICTURES[numPicture];

  for(String line : picture) {
    //напечатать line
  }
}
Здесь каждый уровень распечатывается через свой свич-кейс, а значит это трижды индусский код
public static void printHangman() {
  if (difficulty == EASY) {
    switch (counterOfMistakes) {
      case 0 -> System.out.println(Mistakes.ZERO_MISTAKE_EASY_DIFFICULT);
      //миллион строк
    }
  } else if (difficulty == MEDIUM) {
    switch (counterOfMistakes) {
      case 0 -> System.out.println(Mistakes.ZERO_MISTAKE_MEDIUM_DIFFICULT);
      //миллион строк
    }
  } else if (difficulty == HARD) {
    switch (counterOfMistakes) {
      case 0 -> System.out.println(Mistakes.ZERO_MISTAKE_HARD_DIFFICULT);
      //миллион строк
    }
  }
}

Это игра, а в любой игре должны быть методы, которые проверяют состояние игры

if(isВыиграл()) {
  //действия при выигрыше
} else if(isПроиграл()) {
  //действия при проигрыше
}
//...

private static boolean isВыиграл() {...}
private static boolean isПроиграл() {...}
Сейчас таких методов нет, а проверка состояния игры происходит косвенно
public static void mainGameLoop() {
  while (counterOfMistakes != Math.ceil((double) MAX_COUNT_OF_MISTAKES / difficulty) && !HIDDEN_WORD.equals(WORD)) {...}
  //...
}

//ПРАВИЛЬНО:
public static void mainGameLoop() {
  while (inGame()) {...}
  //...
}

private static boolean inGame() {
 return !isВыиграл() && !isПроиграл();
}

enum Mistakes, картинки хангмана
-Хранит сразу три набора картинок для разных уровней сложности.
Нужно сделать три енама, каждый из которых будут хранить свой набор картинок.

-В картинках должны храниться только картинки, а не картинки+текст
EIGHT_MISTAKE_EASY_DIFFICULT("""
  ＿＿＿＿
  |    |
  |    o
  |   /0\\
  |   / \\
  |
  YOU DEAD!\
  """);

Печатать картинки и сопроводительный текст- разные задачи!
Например так
printHangman();
if(isПроиграл()) {
  //выполнить действия при проигрыше в том числе напечатать "YOU DEAD!"
}
===
Архитектура
А1. Итак, в игре есть уровни сложности, но реализованы они не очень удачно- мегаенам с одной стороны и тройной свичкейс- с другой.
a) Оставаясь в рамках процедурного стиля можно сделать чуть лучше. Допустим, мы решили, что у нас есть три уровня сложности
public enum Level {
  EASY, MEDIUM, HARD
}
Чем они будут отличаться? Сейчас уровни сложности это цифры, которые принимают участие в хитрой формуле, где высчитывается максимальное количество ошибок на каждом уровне
Math.ceil((double) MAX_COUNT_OF_MISTAKES / difficulty)
Таким образом в результате рассчетов этот параметр равен: 6 ошибок EASY, 4 ошибки MEDIUM, 3 ошибки HARD.
б) Избавимся от промежуточных рассчетов, засуним эти параметры сразу в енам
public enum Level {
  EASY(6),
  MEDIUM(4),
  HARD(3);
  private final int maxMistakes;

  Level(int maxMistakes) {
    this.maxMistakes = maxMistakes;
  }

  public int getMaxMistakes() {
    return maxMistakes;
  }
}
в) Получение номера уровня сложности от юзера
System.out.printf("Выберите уровень сложности введя цифру: \n%d - Легко \n%d - Средне \n%d - Сложно ",
  Level.EASY.ordinal(),
  Level.MEDIUM.ordinal(),
  Level.HARD.ordinal()
);
int numLevel = //ввод номера уровня от юзера
if(numLevel < Level.EASY.ordinal() || numLevel > Level.HARD.ordinal()) {
  //ввели неправильный номер уровня
}

//РЕЗУЛЬТАТ:
Выберите уровень сложности введя цифру: 
0 - Легко 
1 - Средне 
2 - Сложно 
г) Перевод номера в уровень сложности
Level currentLevel = Level.values()[numLevel];
И все, получили енам-текущий уровень и везде можно его юзать.
д) Проверка на проигрыш
if(counterOfMistakes == currentLevel.getMaxMistakes()) {
  //таки проиграл
}
е) Распечатка картинок хангмана
класс ХангманРендерер{

  private static final String[] MEDIUM_PICTURES = {
      """
      ＿＿＿＿
      |    |
      |    o
      |   /0\\
      |   /
      |\
      """),
      //oth's pic's
  };

  public static void print(Level level, int pictureNumber) {
    String[] pictures = switch(level) {
      EASY -> EASY_PICTURES[];
      MEDIUM -> MEDIUM_PICTURES[];
      //...
    }
     String picture = pictures[pictureNumber];
    //распечатать picture
  }
}
ё) И тогда распечатка хангмана выглядит совсем просто
public class Hangman {
  private static void какойтоМетод() {
    //...
    ХангманРендерер.print(currentLevel, counterOfMistakes);
    //...
  }
}
В ООП стиле это делается еще красивее, но и в процедурном выглядит неплохо.
А2. Ну и бонусом лайфхак про вывод подсказки
Выберите уровень сложности введя цифру от 2 до 4: 
Легко (2)
Средне (3)
Сложно (4)
4
[п, о, д, н, о, с, ч, и, ц, а]
Это больше ООП стиль, но да ладно.
Допустим, мы пилим виселицу и делаем для ее запуска классический майн
public class Main {
  public static void main(String[] args) {
    Словарь словарь = new Словарь(ПУТЬ_К_ФАЙЛУ);
    String word = словарь.get(); 
    Game game = new Game(word);
    game.start();
  }
}
Все хорошо, но для тестирования мы должны знать отгадываемое слово- чтобы быстро выиграть и проиграть и посмотреть как при этом поведет себя программа.
Тогда делаем второй тестовый майн
public class ТестMain {
  public static void main(String[] args) {
    Словарь словарь = new Словарь(ПУТЬ_К_ФАЙЛУ);
    String word = словарь.get(); 
    System.out.println("Подсказка: " + word);
    Game game = new Game(word);
    game.start();
  }
}
Этот майн можно поместить в гитигнор и тогда он не попадет в репозиторий, а значит качающий игру юзер не будет видеть подсказок.
А разработчик c помощью этого тестового майна будет тестить игру.
---
Вывод: изучить конвенцию кода. В целом для процедурного стиля весьма неплохо. Понравилось, что методы в основном небольшие.
#ревью #виселица #уровнисложности #gamelevel #архитектура #читаетфайл