code review java – Simple juego del ahorcado en Java

Pregunta:

Me estoy enseñando Java a mí mismo y tenía algunas preguntas básicas sobre qué tan bien estoy escribiendo mi código para mejorar la legibilidad. Siento que a veces solo estoy escribiendo código que funciona, y no estoy tan orgulloso de mí mismo cuando he terminado. No te estoy pidiendo que reescribas mi código; Solo estoy pidiendo algunos consejos sobre cómo optimizar mi código para mejorar la legibilidad.

import javax.swing.*;
import java.awt.event.*;
import java.awt.*;

public class Hangman implements ActionListener{
    JFrame frame;
    JTextField userInput;
    JLabel textContents;
    String word = "horse";
        int correctGuesses;
    int incorrectGuesses;
    String guessesLeft;
    boolean lose = false;
    StringBuilder wordsGuessedCorrectly;
    JLabel letters;


    Hangman() {
        incorrectGuesses = 0;
        correctGuesses = 0;
        guessesLeft = "You have " + (6 - incorrectGuesses) +" chances left to guess a " + word.length() + " letter word";
        wordsGuessedCorrectly = new StringBuilder();

        frame = new JFrame("A Hangman Game");
        frame.setSize(300,125);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setLayout(new GridLayout(3, 1));

        userInput = new JTextField();
        userInput.addActionListener(this);
        frame.add(userInput);

        textContents = new JLabel();
        textContents.setText(guessesLeft);
        textContents.setHorizontalAlignment(JLabel.CENTER);
        textContents.setBorder(BorderFactory.createLineBorder(Color.BLUE));
        frame.add(textContents);

        letters = new JLabel("The letters you guess correctly go down here!");
        letters.setHorizontalAlignment(JLabel.CENTER);
        letters.setBorder(BorderFactory.createLineBorder(Color.RED));
        frame.add(letters);

        frame.setVisible(true);


    }

    String seperateLetter(String a, int x, int y) {
        return a.substring(x, y);
    }



    void testLetter(String a) {
        if (word.contains(a) && wordsGuessedCorrectly.toString().contains(a) == false && correctGuesses != (word.length() - 1)) {
            correctGuesses++;
            wordsGuessedCorrectly.append(a);
            letters.setText(wordsGuessedCorrectly.toString());
            textContents.setText("Correct Guess!");
        }
        else if (word.contains(a) && wordsGuessedCorrectly.toString().contains(a)) {
            textContents.setText("You've already guessed this letter!");
        }
        else if (word.contains(a) && wordsGuessedCorrectly.toString().contains(a) == false && correctGuesses == (word.length() - 1)) {
            lose = true;
            textContents.setText("You Win!");
            wordsGuessedCorrectly.append(a);
            letters.setText(wordsGuessedCorrectly.toString());
        }
        else if (word.contains(a) == false && incorrectGuesses == 5){
            textContents.setText("You lose!");
            lose = true;
        }
        else {
            incorrectGuesses++;
            textContents.setText("Incorrect Guess! You have " + (6 - incorrectGuesses)  + " left.");
        }
    }

    public void actionPerformed(ActionEvent ae) {
        if (lose == false) {
            testLetter(seperateLetter(userInput.getText(), 0 , 1));
            userInput.setText("");
        }
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                new Hangman();
            }
        }); 
    }
}

Respuesta:

Rápidamente, aquí tienes algunos comentarios:

  • Creo que su código de configuración está bien (donde está creando los JFrames, etc.), es muy complicado, pero eso es Java.

  • Pero no deberías poner esa lógica en el constructor. Ponlo en un método init() , llamado después de la construcción.

  • Utilice declaraciones de alcance, es decir. hacer que el Hangman() constructor public Hangman() . Lo mismo con void testLetter ¿este método está destinado a ser público o privado? Hágalo explícito para que otros (¡y usted mismo, en seis meses!) Sepan cómo se debe usar el código.

  • Considere separar su lógica Swing / GUI y la lógica de su juego en diferentes clases. es decir, algo como HangmanApp (su clase principal) contiene un HangmanGui y un HangmanLogic .

  • Tienes números mágicos a lo largo de tu código. Por ejemplo, esta línea es particularmente mala: textContents.setText("Incorrect Guess! You have " + (6 - incorrectGuesses) + " left."); ¿Qué pasa si desea cambiar el número de conjeturas? Idealmente, estas configuraciones se establecerían en un archivo de propiedades, pero para una aplicación pequeña, establecerlas en la parte superior de la clase está bien.

  • Tu if else ramificar en testLetter da miedo. Tal como está actualmente, no puedo ver de inmediato para qué sirve cada rama. Los comentarios ayudarían en cada rama, pero movería la lógica 'prueba si la letra se ajusta' y la lógica 'qué debo hacer si la letra es correcta / incorrecta' a sus propios métodos privados.

p.ej. algo como:

void testLetter(String a) {


     if (evaluateGuess(a)){   //returns true if guess is correct
         handleCorrectGuess(a); 
     }
     else (
         handleIncorrectGuess(a); 
     }

}
  • No es necesario utilizar una comparación == en valores booleanos. Utilice un nombre de variable descriptivo y ! operador en su lugar. es decir. if (!gameLost){... o if(gameStillInProgress){...

Leave a Comment

Your email address will not be published. Required fields are marked *

Scroll to Top

web tasarım