7
\$\begingroup\$

This is a continuation of this question. In the game the player chooses a topic and then gets a random word (or words) from that topic to guess.

enter image description here I've repartitioned the code into objects several times, without changing the player's experience of the game at all. Some partitionings were clearly better than others. In other cases I wasn't sure. My question is: how do you judge the quality of a partitioning? - preferably, before coding. I'm assuming that some of you use diagrams (UML) in the same way as architects explore different floor plans in their drawings. I've had an attempt at a UML diagram for my code. Can you judge the quality from a diagram like that? I'm struggling a bit to understand UML - so the diagram may be pretty flawed! (hints on how to draw UML welcome).

One thing I wrestled with is, whether to make getting a topic and getting a word list part of Game, or as here, part of getting a word. Maybe it doesn't matter?

You can run it at Replit.com here. The code is here at GitHub. Thanks for their comments to Martin Frank and and RoToRa

Any additional comments on the detail of the code, or the partitioning into objects, appreciated.

Ammendment: Changed WordToGuess.getWordsInTopic() to open stream using try with resources:

Path path = Paths.get(Constants.WORD_LIST_DIRECTORY +topic);
try (Stream<String> stream = Files.lines(path)){
 ...

uml Major classes follow:

Main.java

public class Main {
 public static void main (String[] args){
 Game game = new Game();
 }
}

Game.java

import javax.swing.JPanel;
import javax.swing.JOptionPane;
class Game {
 private Progress progress;
 private GUI gui;
 Game() {
 String targetWord = new WordToGuess().getWordToGuess();
 progress = new Progress(targetWord);
 JPanel alphabet = new Alphabet(this);
 gui = new GUI(progress.getProgress(),alphabet);
 gui.setVisible(true);
 }
 void recordGuess(char letter) {
 progress.updateProgress(letter);
 gui.showStatus(progress.getNumberOfWrongGuesses(), progress.getProgress());
 if (progress.playerWins()) {
 displayEndGameMessage("Well done, it was "+progress.getTargetWord()+", another game?");
 }
 if (progress.playerLooses()) {
 displayEndGameMessage("Bad luck, it was "+progress.getTargetWord()+", another game?");
 }
 }
 void displayEndGameMessage(String message){
 int reply = JOptionPane.showConfirmDialog(gui, message, "end", JOptionPane.YES_NO_OPTION);
 if (reply == JOptionPane.YES_OPTION) {
 GUI oldGUI = gui;
 new Game();
 oldGUI.dispose();
 } else {
 System.exit(0);
 }
 }
}

WordToGuess.java

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Random;
import java.util.function.Predicate;
import java.util.stream.Stream;
import static java.util.stream.Collectors.toList;
public class WordToGuess {
 private static final int MAX_WORD_LENGTH = 20;
 private static final int MIN_WORD_LENGTH = 3;
 private final String word;
 private final Predicate<String> wordLengthPredicate =
 s -> s.length() <= MAX_WORD_LENGTH && s.length() >= MIN_WORD_LENGTH;
 WordToGuess() {
 List<String> candidateWords = getCandidateWords();
 int randomIndex = new Random().nextInt(candidateWords.size());
 word = candidateWords.get(randomIndex);
 }
 String getWordToGuess() {
 return word;
 }
 private List<String> getCandidateWords() {
 List<String> candidateWords = defaultWords();
 try {
 String topic = new TopicProvider().getTopic();
 candidateWords = getWordsInTopic(topic);
 }catch(NoWordsListsException e){
 ErrorReporter.missingFiles();
 }catch (TopicWordsMissingException e){
 ErrorReporter.missingWords();
 } catch (WordListDirectoryMissingException e) {
 ErrorReporter.missingDirectory();
 }
 return candidateWords;
 }
 private List<String> getWordsInTopic(String topic) throws TopicWordsMissingException {
 List<String> candidateWords;
 try {
 Path path = Paths.get(Constants.WORD_LIST_DIRECTORY +topic);
 Stream<String> stream = Files.lines(path);
 candidateWords = stream.map(String::toUpperCase)
 .filter(wordLengthPredicate)
 .collect(toList());
 if (candidateWords.size() < 1) {
 throw new TopicWordsMissingException("The file for that topic contained no words");
 }
 } catch (IOException e) {
 throw new TopicWordsMissingException("The file for that topic was missing");
 }
 return candidateWords;
 }
 private static List<String> defaultWords(){
 return List.of("FOXGLOVE", "MICROWAVE","ZOMBIE","PUPPY","RHUBARB","DWARF","BICYCLE",
 "BUZZARD","OWL","CHAFFINCH","KIRIBATI","LIECHTENSTEIN","MOZAMBIQUE");
 }
}

TopicProvider.java

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.function.Function;
import java.util.stream.Stream;
import static java.util.stream.Collectors.toList;
class TopicProvider {
 String getTopic() throws NoWordsListsException, WordListDirectoryMissingException {
 List<String> categories = getTopics();
 return getPlayerChoice(categories);
 }
 private List<String> getTopics() throws NoWordsListsException, WordListDirectoryMissingException {
 Path path = Paths.get(Constants.WORD_LIST_DIRECTORY);
 List<String> topics;
 try {
 Stream<Path> stream = Files.list(path);
 topics = stream.filter(Files::isRegularFile)
 .map(toTopic)
 .collect(toList());
 if (topics.size() < 1) {
 throw new NoWordsListsException("Word lists missing");
 }
 } catch (IOException e) {
 throw new WordListDirectoryMissingException("Word lists directory missing");
 }
 return topics;
 }
 private final Function<Path, String> toTopic = path -> path.getFileName().toString();
 private String getPlayerChoice(List<String> Topics){
 return new TopicDialogue().ShowTopicDialogue(Topics);
 }
}

TopicDialogue.java

import javax.swing.JComboBox;
import javax.swing.JOptionPane;
import java.util.List;
class TopicDialogue {
 private String topic;
 String ShowTopicDialogue(List<String> categoryNames){
 String[] categories = categoryNames.toArray(new String[0]);
 topic = categories[0];
 JComboBox<String> jComboBox = new JComboBox<>(categories);
 jComboBox.addActionListener(e -> topic = (String) jComboBox.getSelectedItem());
 JOptionPane.showMessageDialog(null, jComboBox, "Choose a topic", JOptionPane.PLAIN_MESSAGE);
 return topic;
 }
}

Progress.java

public class Progress {
 private static final String placeHolder = "*";
 private final char[] progress;
 private int wrongGuessCount =0;
 private final String targetWord;
 private final int MAX_BAD_GUESSES = 11;
 Progress(String targetWord){
 this.targetWord=targetWord;
 String progressString = targetWord.replaceAll("[A-Z]", placeHolder);
 progress = progressString.toCharArray();
 }
 String getProgress(){
 return new String(progress);
 }
 String getTargetWord(){
 return targetWord;
 }
 boolean playerWins(){
 return !new String(progress).contains(placeHolder);
 }
 boolean playerLooses(){
 return (wrongGuessCount > MAX_BAD_GUESSES);
 }
 int getNumberOfWrongGuesses(){
 return wrongGuessCount;
 }
 void updateProgress(char guess) {
 if (isGoodGuess(guess)) {
 for (int n = 0; n < targetWord.length(); n++) {
 if (targetWord.charAt(n) == guess) {
 progress[n] = guess;
 }
 }
 } else {
 wrongGuessCount++;
 }
}
 boolean isGoodGuess(char guess){
 return targetWord.contains(Character.toString(guess));
 }
}

GUI.java

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.JLabel;
import javax.swing.BoxLayout;
import java.awt.Dimension;
public class GUI extends JFrame {
 private JLabel progressLabel;
 private GibbetLabel gibbetLabel;
 GUI(String prog, JPanel alphabet) {
 JPanel container = new JPanel();
 container.setLayout(new BoxLayout(container, BoxLayout.Y_AXIS));
 add(container);
 setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 gibbetLabel = new GibbetLabel();
 container.add(gibbetLabel);
 progressLabel = new ProgressLabel(prog);
 container.add(progressLabel);
 container.add(alphabet);
 setTitle("Hangman");
 setSize(new Dimension(350, 450));
 setLocationRelativeTo(null);
 setVisible(true);
 }
 void showStatus(int count, String prog){
 progressLabel.setText(prog);
 gibbetLabel.updateGibbetImage(count);
 }
}

Alphabet.java

import javax.swing.JPanel;
import javax.swing.JButton;
class Alphabet extends JPanel{
 Alphabet(Game game){
 for(char c = 'A'; c <= 'Z'; ++c){
 JButton button = new JButton(Character.toString(c));
 button.addActionListener(e -> {
 game.recordGuess(e.getActionCommand().charAt(0));
 button.setEnabled(false);
 });
 add(button);
 }
 }
}

ProgressLabel.java

import javax.swing.JLabel;
import java.awt.Font;
import java.awt.Component;
public class ProgressLabel extends JLabel {
 ProgressLabel(String prog){
 setText(prog);
 setFont(new Font("Monospaced", Font.PLAIN, 20));
 setAlignmentX(Component.CENTER_ALIGNMENT);
 }
}

GibbetLabel.java

import javax.swing.*;
import java.awt.*;
import java.io.File;
import java.util.ArrayList;
public class GibbetLabel extends JLabel {
 private static final int IMAGE_COUNT = 13;
 private ArrayList<File> gibbetImages = new ArrayList<>();
 GibbetLabel() {
 for (int i = 0; i < IMAGE_COUNT; i++) {
 try {
 gibbetImages.add(new File("images/" + i + ".png"));
 } catch (Exception e) {
 e.printStackTrace();
 }
 }
 setIcon(new ImageIcon(String.valueOf(gibbetImages.get(0))));
 setAlignmentX(Component.CENTER_ALIGNMENT);
 }
 void updateGibbetImage(int count){
 setIcon(new ImageIcon(String.valueOf(gibbetImages.get(count))));
 }
}
jmizv
1351 gold badge1 silver badge8 bronze badges
asked Feb 2, 2022 at 14:34
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

It's been a while since this question was asked, so I'm going to have some general points too.

Inheritance design

Generally, projects should at least have an initial UML ready. That being said, to an extent is project-specific, and also on who gets to decide.

  • For non-exception classes, the typical use-case for introducing a subclass is switching on a type-field, while the case to eliminate it is when subclasses don't have much beyoynd their parent.
  • Exceptions are a little different. Here you usally want subclasses, so people don't have to disect the message or wade through an errorCode field. However, errorCode-style fields can be usefull if otherwise there would be a huge number of subclasses (I'm looking at you, SQLException)
  • ProgressLabel does not extend JLabel with extra functionality, it can be eliminated.

Class design

  • find a new home for the single constant in Constants, then delete that class.
  • ErrorReporter needs to become aware of the GUI Frame. null-parented dialogs have a tendency to show up at the wrong z-index, leading to user confusion. TopicDialogue ditto
  • GibbetLabel relies on the assets being files in the filesystem, and will break if things are jarred up. See e.g. this SO question for a way (and its caveats)(also, that catch block there does nothing, unless we're out of memory)
  • I've been told that in the C(++) world, Game game = new Game(); is how things are done. In java, this needs another method, possibly launch().
  • WordToGuess.wordLengthPredicate does not need access to instance fields and can become static. TopicProvider.toTopic and Progress.MAX_BAD_GUESSES ditto.
  • WordToGuess shouldn't yell at the user about missing/broken wordlists before each game.
  • WordToGuess should become aware of file extensions, so you're not attempting to read binary files into a wordlist.

General points

  • I didn't find a build system in your Github. These days, most projects use Maven or Gradle.
  • Classes should not go in the toplevel package. In the Java World, it is common to use a reverse tld location, so I used com.stackexchange.cr.cr273670 as the base package name. The toplevel package is realy only for things that fit into one class - or at most a single source file.
  • A bunch of your stuff is missing access modifiers. The go-to modifyer should be public, with private being used where the former is undesireable. The other two are exotic usages. The fields in Game can become final (GUI ditto).
  • Progress has inconsistent identing.
  • The current startup flow breaks Swing's Threading Policy by creating GUI objects off the EDT. Take a look at EventQueue.invokeLater()
  • WordToGuess has some deplorable Exception handling. Generally, you want to record a caught Exception as the cause when throwing another exception. Also, IOExceptions can be caused by more than missing files (file permission issiues, or the user exceeding their file handle limit)

Naming

  • TopicDialogue -> TopicDialog
  • TopicDialogue.ShowTopicDialogue -> TopicDialogue.showTopicDialog

Future expansion

  • Make TopicProvider rely on ServiceLoader so users can supply their own providers.
answered Sep 7 at 5:51
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.