HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Complete Hangman game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
completegamehangman

Problem

I would like to improve code readability and reduce the complexity of my code to improve the maintainability of the source code.

If you have any tips, please say so. I was also wondering if it would be a good feature to add a 2-player option to this game. Currently, the game runs off of a word list in which it randomly selects a word.

```
package com.game;

import java.awt.BorderLayout;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;

import javax.swing.BorderFactory;
import javax.swing.BoxLayout;
import javax.swing.ImageIcon;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.JTextField;

class GameStructure {
private String[] wordList = { "computer", "java", "activity", "alaska",
"appearance", "javatar", "automobile", "falafel", "birthday",
"canada", "central", "character", "chicken", "chosen", "cutting",
"daily", "darkness", "shawarma", "disappear", "driving", "effort",
"establish", "exact", "establishment", "fifteen", "football",
"foreign", "frequently", "frighten", "function", "gradually",
"hurried", "identity", "importance", "impossible", "invented",
"italian", "journey", "lincoln", "london", "massage", "minerals",
"outer", "paint", "particles", "personal", "physical", "progress",
"quarter", "recognise", "replace", "rhythm", "situation",
"slightly", "steady", "stepped", "strike", "successful", "sudden",
"terrible", "traffic", "unusual", "volume", "yesterday" };
private JTextField tf;
private JLabel jlLetsUsed;
static String letter;
static int[] wordLength = new int[64];
static int level = (int) (Math.random() * 64);// random word
static JFrame frame = new JFram

Solution

Review these general concepts for a Hangman game. Notice how LazySloth13's hangman game and your hangman game are not reusable. That is, there is no way to take your vision of hangman and get the vision from LazySloth13, or vice-versa. Yet they are both hangman, and a simple version at that.

Object-Oriented Programming has reusability as a core goal. A concept that helps achieve that goal is the Open-Closed Principle, along with other SOLID practices. Both versions of hangman violate most of these principles in similar ways.

Most predominately, the biggest violation is that there is no separation between the game's logic and the game's presentation. What if you wanted to port your game to the Web? A mobile device? A custom piece of hardware? As it stands, a lot of code would have to be re-written.

Let's take a closer look at how the code violates the SOLID principles.

Single Responsibility

Each class should be limited to a single reason for changing it. In the given code, if I wanted to change the number of guesses or the list of words, there is only a single class. This tight coupling means that it is possible to affect the code for guesses by changing the code for the list of words. Ideally, we'd want this to be impossible.

This points to the first change: create a HangmanDictionary class that is responsible for all aspects of the game's dictionary:

  • Set the dictionary source (could be an array, an input stream, a database, or HTTP connection).



  • Load a dictionary (for a given language).



  • Return a randomly selected word.



That's it. A dictionary doesn't know about guessing words. The mechanism behind guessing words is part of the rules.

Open-Closed Principle

Once a class is finished (i.e., it performs the single responsibility required), it should be closed from further modifications. In the code, it is impossible to change the font without changing the class itself:

jl.setFont(new Font("Rockwell", Font.PLAIN, 20));
    tf.setFont(new Font("Rockwell", Font.PLAIN, 20));
    jlLetsUsed.setFont(new Font("Rockwell", Font.PLAIN, 20));
    jlLines.setFont(new Font("Rockewell", Font.PLAIN, 20));


This is due to the tight coupling between the logic and the view, which points to the second change: create a class that is responsible for displaying the results of the guesses, called HangmanView. In it you might see:

protected Font getTextFont() {
  return new Font( "Rockwell", Font.PLAIN, 20 );
}


If I wanted to change the font, now I could create a subclass of HangmanView and override a single method getTextFont(). Your original code would remain untouched, thereby ensuring to a great extent that my code would not introduce bugs in your code. (My code could still be buggy, but that's a different issue.)

Liskov Substitution

From CS 3443,


objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.

I cannot substitute my own subclass of GameStructure. This is because there is only one method to override: window(), which contains the entirety of the program.

The third change, which should be reasonably apparent by this time, is to separate the functionality of the window() method into various classes, each with its own single responsibility. What if I wanted to change how the restart functionality worked? Currently:

GameStructure restart = new GameStructure();
                level = (int) (Math.random() * 64);
                restart.window();


Note that it should not be required to create a new instance of the entire game just to reset it. Resetting "hangman" means telling the view to reset, telling the rules to reset (e.g., the number of guesses), and then picking a new random word. The above code should be included in a method such as:

public void reset() {
  getView().reset();
  getRules().setGuessWord( getDictionary().getRandomWord() );

  // Setting the new word to guess could call reset as it makes
  // little sense, in Hangman, to pick a new word but keep the
  // current tally of incorrect guesses.
  getRules().reset();
}


Now, if I wanted to change the way resetting the works, I can simply override the reset() method. (For example, what if I wanted to make a difficulty level that, in English, chooses words without the letters R S T L N E?)

Interface Segregation

There's not much to say here except that there is no code that a client class can reuse. My answer to a similar question has details on how to separate the interfaces.

Dependency Inversion

Martin Fowler identified three types of dependency injection:

  • Interface - clients must implement a specific interface.



  • Setter - expose setter methods for injecting dependencies.



  • Constructor - clients inject dependencies upon instantiation.



I, personally, feel that classes should "just work" upon instantiation and tend to opt for setter injection. This implies a fourth change: use class-scoped variables that can be c

Code Snippets

jl.setFont(new Font("Rockwell", Font.PLAIN, 20));
    tf.setFont(new Font("Rockwell", Font.PLAIN, 20));
    jlLetsUsed.setFont(new Font("Rockwell", Font.PLAIN, 20));
    jlLines.setFont(new Font("Rockewell", Font.PLAIN, 20));
protected Font getTextFont() {
  return new Font( "Rockwell", Font.PLAIN, 20 );
}
GameStructure restart = new GameStructure();
                level = (int) (Math.random() * 64);
                restart.window();
public void reset() {
  getView().reset();
  getRules().setGuessWord( getDictionary().getRandomWord() );

  // Setting the new word to guess could call reset as it makes
  // little sense, in Hangman, to pick a new word but keep the
  // current tally of incorrect guesses.
  getRules().reset();
}
public class HangmanDictionary {
  // Objects should "just work", so give some initially valid data.
  private String[] words = { "computer", ... };

  public void load( File file ) {
    load( new FileInputStream( file ) );
  }

  public void load( InputStream i ) { 
    String wordList[];

    // ... reads the words from an input stream

    // Now replace the words used for the game.
    setWords( wordList );
  }

  protected void setWords( String[] words ) {
    assert words != null;

    if( words.length > 0 ) {
      this.words = words;
    }
  }
}

Context

StackExchange Code Review Q#38187, answer score: 3

Revisions (0)

No revisions yet.