0
\$\begingroup\$

Last time we posted here we were overwhelmed by the useful feedback. We've had another go at OOP/programming - this time in Java. Very different than Python! As before we have lots of concerns at the macro level (is our choice of objects any good?) and the micro level. We'd be grateful for any suggestions for improvement in style, technique and overall design.

You can run the code here: replit and the source and support files are here: github

Thanks again to @ferada @Alex Waygood @m-alorda and others who gave us such good feedback on the Python version.

Game.java

import java.awt.Dimension;

import javax.swing.JFrame;
import javax.swing.BoxLayout;
import javax.swing.JOptionPane;

/**
 * Game has two JPanels. "Gibbet" displays the state of the hanging. "Guesses" displays the
 * progress towards guessing the word/term and the alphabet buttons that allow the player
 * to choose letters.
 */
class Game extends JFrame {

    Gibbet gibbet;
    private int badGuessCount = 0;
    static final int MAX_BAD_GUESSES = 12;
    WordLists wordLists;

    Game(){
        setTitle("hangman ");
        setLayout(new BoxLayout(this.getContentPane(), BoxLayout.Y_AXIS));
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setSize(new Dimension(350, 460));

        gibbet = new Gibbet();
        add(gibbet);
        setVisible(true);

        wordLists = new WordLists(this);
        setTitle("Hangman: "+ wordLists.getCategory());
        Guesses guesses = new Guesses(wordLists.getWord(), this);
        add(guesses);
        setVisible(true);
    }

    void badGuess(){
        gibbet.nextImage();
        badGuessCount++;
        if (badGuessCount >= MAX_BAD_GUESSES){
            gameLost();
        }

}

    void gameWon(){
        String message = "Well done, it was "+ wordLists.getWord() +", another game?";
        endGame(message);
    }

    private void gameLost(){
        String message = "Bad luck, it was "+ wordLists.getWord() + ", another game?";
        endGame(message);
    }

    void endGame(String message){
        int reply = JOptionPane.showConfirmDialog(this, message, "end", JOptionPane.YES_NO_OPTION);
        if (reply == JOptionPane.YES_OPTION){
            this.dispose();
            new Game();
        }else{
            System.exit(0);
        }
    }



    public static void main (String[] args){
        new Game();
    }
}

WordLists.java

import java.io.IOException;
import java.io.File;
import java.io.FileNotFoundException;

import javax.swing.JComboBox;
import javax.swing.JOptionPane;
import javax.swing.JFrame;

import java.util.Scanner;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import java.util.Objects;

/**
 * Text files contain lists of words or terms. The filename is used as a category for the words e.g.
 * the file "birds" contains a list of bird names. The player chooses from a list of filenames
 * and a random word/term from that list becomes the target word for the game. Words/terms can
 * contain spaces, hyphens and other punctuation. The maximum length is 20 characters (sufficient
 * for "Johann Sebastian Bach" or "Lennon and McCartney". The minimum length is three characters
 * (sorry "Ye"). If there's a problem finding a valid word list, the program falls back to a
 * hard-coded default list.
 */
public class WordLists {
    private String category;
    private static final String wordListDirectory = "wordLists/";
    private final String targetWord;
    private static final int maxWordLength = 20;
    private static final int minWordLength = 3;

    WordLists(JFrame frame){
        ArrayList<String> words;
        try {
            chooseCategory(frame);
            words = getWordsInCategory(frame);
        }catch (IOException e){
            JOptionPane.showMessageDialog(frame, "No word lists found, using default words instead");
            words=loadDefaultWords();
        }
        Random random = new Random();
        String word = words.get(random.nextInt(words.size()));
        targetWord = word.toUpperCase().trim();
    }

    String getCategory(){
        return category;
    }

    String getWord(){
        return targetWord;
    }

    private void chooseCategory(JFrame  frame) throws IOException {
        String[] fileList = getCategories();
        if (fileList.length>0) {
            category = fileList[0];
            JComboBox<String> jComboBox = new JComboBox<>(fileList);
            jComboBox.addActionListener(e -> category = Objects.requireNonNull(jComboBox.getSelectedItem()).toString());
            JOptionPane.showMessageDialog(frame, jComboBox, "Word list",
                    JOptionPane.QUESTION_MESSAGE);
        } else {
            throw new IOException("No word lists found");
        }
    }

    private String[] getCategories(){
        var folder = new File(wordListDirectory);
        File[] files = folder.listFiles();
        List<String> fileList = new ArrayList<>();
        for (File file : Objects.requireNonNull(files)) {
            if (file.isFile()) {
                fileList.add(file.getName());
            }
        }
        return fileList.toArray(new String[0]);
    }

    private ArrayList<String> getWordsInCategory(JFrame  frame){
        ArrayList<String> words = new ArrayList<>();
        File file = new File(wordListDirectory+category);
        try {
            Scanner sc = new Scanner(file);
            while(sc.hasNextLine()) {
                String word = sc.nextLine();
                if (word.length() >= minWordLength && word.length() <= maxWordLength){
                    words.add(word);
                }
            }
        } catch (FileNotFoundException e) {
            JOptionPane.showMessageDialog(frame, "Text file for words missing, using default words instead");
            words=loadDefaultWords();
        }
        if (words.size()<1){
            JOptionPane.showMessageDialog(frame, "Selected list empty, using default words instead");
            words=loadDefaultWords();
        }
        return words;
    }

    private ArrayList<String> loadDefaultWords(){
        category = "default list";
        ArrayList<String> words = new ArrayList<>();
        words.add("FOXGLOVE");
        words.add("MICROWAVE");
        words.add("ZOMBIE");
        words.add("PUPPY");
        words.add("RHUBARB");
        words.add("DWARF");
        words.add("BICYCLE");
        words.add("BUZZARD");
        words.add("OWL");
        words.add("CHAFFINCH");
        words.add("KIRIBATI");
        words.add("LIECHTENSTEIN");
        words.add("MOZAMBIQUE");
        return words;
    }

}

Gibbet.java

import java.awt.CardLayout;
import java.awt.image.BufferedImage;

import javax.imageio.ImageIO;
import java.io.File;
import java.io.IOException;

import javax.swing.ImageIcon;
import javax.swing.JPanel;
import javax.swing.JLabel;
import javax.swing.JOptionPane;

class Gibbet extends JPanel {
    /**
     * This class is responsible for displaying the state of the hanging. With each bad
     * guess the image of the hanging is advanced by one.
     */

    private int currentCard = 0;
    private final CardLayout cardLayout = new CardLayout();
    private final JPanel cardPanel = new JPanel();
    private static final int IMAGE_COUNT = Game.MAX_BAD_GUESSES+1;

    Gibbet (){
        cardPanel.setLayout(cardLayout);
        for (int i = 0; i < IMAGE_COUNT; i++ ) {
            try{
                BufferedImage myPicture = ImageIO.read(new File("images/" + i + ".png"));
                cardPanel.add(new JPanel().add(new JLabel(new ImageIcon(myPicture))), ""+i);
            }
            catch (IOException e){
                JOptionPane.showMessageDialog(null, "Image file for gibbet missing, exiting...");
                System.exit(0);
            }
        }
        add(cardPanel);
    }

    void nextImage(){
        if (currentCard < IMAGE_COUNT){
            currentCard += 1;
            cardLayout.show(cardPanel, "" + (currentCard));
        }
    }
}

Guesses.java

import java.awt.Font;
import java.awt.Component;
import java.awt.Dimension;

import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.BoxLayout;
import javax.swing.JButton;

class Guesses extends JPanel{
    /**
     * The target word/term is represented by a template with each letter replaced by a placeholder.
     * Buttons represent each letter of the alphabet. When a correct letter is selected the placeholder
     * is replaced. Wrong guesses and wins are passed back to Game.
     */
        private char[] template;
        private JLabel templateLabel;
        private static final char placeHolder = '*';


    Guesses(String word, Game game) {
        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));
        setPreferredSize(new Dimension(350, 200));
        createTemplate(word);
        createTemplateLabel();
        add(templateLabel);
        addAlphabetButtons(word, game);
    }

    private void addAlphabetButtons(String word, Game game){
        JPanel alphabetButtons = new JPanel();
        for (char c = 'A'; c <= 'Z'; ++c) {
            JButton button = new JButton(Character.toString(c));
            button.addActionListener(e -> {
                 button.setEnabled(false);
                 char selectedCharacter = e.getActionCommand().charAt(0);
                 if (word.contains(Character.toString(selectedCharacter))) {
                     updateTemplate(word, selectedCharacter);
                     if (checkForWin(word)) {
                         game.gameWon();
                     }
                 }else{
                     game.badGuess();
                 }
            });
            alphabetButtons.add(button);
        }
        add(alphabetButtons);
    }

    private void createTemplate(String word) {
        template = word.toCharArray();
        for (int i = 0; i < word.length(); i++) {
            if (Character.isLetter(word.charAt(i))) {
                template[i] = placeHolder;
            }
        }
    }

    private void createTemplateLabel(){
         templateLabel = new JLabel(new String(template));
         templateLabel.setFont(new Font("Monospaced", Font.PLAIN, 20));
         templateLabel.setAlignmentX(Component.CENTER_ALIGNMENT);
    }

    private void updateTemplate (String word, char guess) {
        for (int n = 0; n < word.length(); n++) {
            if (word.charAt(n) == guess) {
                template[n] = guess;
                templateLabel.setText(new String(template));

            }
        }
    }

    private boolean checkForWin (String word){
        boolean win = true;
        for(int n = 0; n < word.length(); n++){
            if (template[n] == placeHolder) {
                win = false;
                break;
            }
        }
        return win;
    }
}

Sample word list "scientists"

Charles Darwin
Max Planck
Albert Einstein
Michael Faraday
Richard Feynmann
Linnaeus
Lavoisier
Aristotle
Marie Curie
Isaac Newton
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Incorporating advice from an answer into the question violates the question-and-answer nature of this site. You could post improved code as a new question, as an answer, or as a link to an external site - as described in I improved my code based on the reviews. What next?. I have rolled back the edit, so the answers make sense again. \$\endgroup\$ Jan 23 at 10:13

1 Answer 1

2
\$\begingroup\$

The biggest problem in my opinion is a lack of separation of GUI and logic. Specifically a "non-Swing" class containing the all game logic would make this cleaner. And even if you keep Game as a JFrame, some game logic is in Guesses which belongs in Game (the if inside the button action listener, and checkForWin).

Some other random thoughts:

  • WordLists should probably be separated into GUI and file loading logic.

  • You are basically tracking the number of bad guesses twice, once in Game.badGuessCount and once in Gibbet.currentCard.

  • Some fields are lacking an access modifier (private) and several should be final (for example, gibbet and wordLists in Game). And wordLists in Game doesn't need to be a field.

  • The fields should be ordered better. For example, static fields before instance fields. static final fields should be named uppercase.

  • In WordLists words=loadDefaultWords(); shouldn't be repeated, but done at a single place.

  • Guesses should probably be split into two separate components, one for the word, and one for the buttons.

  • You should consider using some more modern APIs, for example, java.nio for file access, Streams for working with lists, List.of for the default word list.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for this feedback @RoToRa. I've changed the initial code that gets a random word: separated out the GUI, used streams and List.of. Thought it best to confirm I'm heading in the right direction, before reworking the rest. I've posted it as a new question here codereview.stackexchange.com/questions/273278/… \$\endgroup\$
    – scripta
    Jan 23 at 16:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.