Gula1507 - HangmanGame.md

https://github.com/Gula1507/HangmanGame
Ревью от Алексея

Игра сделана частично.

Недостатки реализации:

1. При старте игры не видно виселицы

Do you like to start the game (Y/N) ? y Starting the game... The word to guess: *********** Enter the letter

2. Нет списка правильных и неправильных букв, которые вводили
3. Можно ввести символ не букву, а например цифру, и это будет считаться допустимым вводом
4. Можно ввести не одну букву, а целое слово и это будет считаться допустимым вводом

Хорошо:

1. Нейминг(в большинстве случаев)
2. Простой понятный алгоритм

Замечания:

1. Нейминг:

Не нужно указывать в названии методов тип данных, который метод возвращает:

public static ArrayList<String> generateWordsList() {...}
//ПРАВИЛЬНО:
public static ArrayList<String> generateWords() {...}

Метод обещает одно, делает другое. Обещает найти совпадение букв, а на самом деле создает хитрую строку и возвращает ее.

public static String findLetterMatch(char inputLetter, String wordToGuess, String customerWord) {
  StringBuilder result = new StringBuilder(customerWord);
  for (int i = 0; i < wordToGuess.length(); i++) {
    if (wordToGuess.charAt(i) == inputLetter) {
      result.setCharAt(i, inputLetter);
    }
  }
  return result.toString();
}

2. Магические числа и слова. Сздавай константы:

while (!customerWord.equals(wordToGuess) && counter < 13) {
  //...
}
//ПРАВИЛЬНО:
private static final int DEAD_LEVEL = 13;
  //...
while (!customerWord.equals(wordToGuess) && counter < DEAD_LEVEL) {
  //...
}

while (!reply.equals("Y") && !reply.equals("N")) {
  System.out.println("Do you like to start the game (Y/N) ?");
  reply = sc.nextLine().toUpperCase();
  switch (reply) {
    case "N":
      //...      
    case "Y":
      //...      
}

//ПРАВИЛЬНО:
private static final String KEY_YES = "Y";
private static final String KEY_NO = "N";

while (!reply.equals(KEY_YES) && !reply.equals(KEY_NO)) {
  System.out.printf("Do you like to start the game (%s/%s) ?\n", KEY_YES, KEY_NO);
  reply = sc.nextLine().toUpperCase();
  switch (reply) {
    case KEY_NO:
      //...      
    case KEY_YES:
      //...      
}

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

while (!customerWord.equals(wordToGuess) && counter < 13) {
  //...
}
//ПРАВИЛЬНО:
while (isМетодСназваниемКотороеВсеОбьясняет()) {
  //...
}

private static boolean isМетодСназваниемКотороеВсеОбьясняет() {
  return !customerWord.equals(wordToGuess) && counter < 13;
}

if (customerWord.equals(wordToGuess)) {
  System.out.println("You are won! The word was " + wordToGuess);
}
//ПРАВИЛЬНО:
if (isWin()) {
  System.out.println("You are won! The word was " + wordToGuess);
}

private static boolean isWin() {
  return customerWord.equals(wordToGuess);
}

4. ArrayList используй через List:

ArrayList<String> generateWordsList(){...}
String generateWordToGuess(ArrayList<String> words){...} 
//ПРАВИЛЬНО:
List<String> generateWordsList(){...}
String generateWordToGuess(List<String> words){...} 

5. Слишком длинный main. Делает много разного, нужно разбить на вспомогательные методы. Пролцедурный стиль не означает, что всю логику можно засунуть в один метод. Для процедурного стиля так же работает принцип единой отвественности(SRP), который в данном случае гласит, что программа должна состоять из набора маленьких методов, каждый из которых будет делать что-то одно.

6. Лишний метод char doToUpperCase(char inputLetter). Его можно заменить обычным Character.toUpperCase(), это будет работать так же.

7. Индусский код в void drawHangman(int counter) Подумай, как можно распечатывать виселицу без использования свич-кейс. Например, картинки можно хранить в массиве, а брать их оттуда по индексу.

8. В одном классе Main находится и основная логика программы и графика: картинки с висельницей + распечатка. Лучше графику и работу с ней перенести в отдельный класс, например Renderer.

Вывод: для первого раза неплохо, но нужно доделать игру.