Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I would like some help refactoring this code. This is to improve some of the nonfunctional attributes of the software to

  1. improve code readability
  2. reduce complexity to improve the maintainability of the source code.

package Game;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;
import javax.swing.*;

class GameStructure {
    private String []wordList = {"computer","java","activity","alaska","appearance","article",
            "automobile","basket","birthday","canada","central","character","chicken","chosen",
            "cutting","daily","darkness","diagram","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];

   public void window() {
       JMenuBar menuBar = new JMenuBar();
       JMenu menu = new JMenu("File");
       menu.setMnemonic(KeyEvent.VK_A);
       menuBar.add(menu);
       JMenuItem menuItem = new JMenuItem("Developer", KeyEvent.VK_T);
    menu.add(menuItem);
    JMenuItem menuItem2 = new JMenuItem("Instructions", KeyEvent.VK_T);
    menu.add(menuItem2);
    JMenuItem menuItem3 = new JMenuItem("Levels", KeyEvent.VK_T);
    menu.add(menuItem3);
    JMenuItem menuItem4 = new JMenuItem("Restart", KeyEvent.VK_T);
    menu.add(menuItem4);
    JMenuItem menuItem5 = new JMenuItem("Exit", KeyEvent.VK_T);
    menu.add(menuItem5);

       ImageIcon ic = new ImageIcon("hangman2.png");
      JFrame gameFrame = new JFrame();
      JPanel bottomRight = new JPanel();
      JPanel bottomLeft = new JPanel();
      JPanel top = new JPanel();
      JPanel bottom = new JPanel();
      JPanel imgPane = new JPanel();
      JPanel panel1 = new JPanel();
      bottom.setLayout(new BoxLayout(bottom, BoxLayout.X_AXIS));
      imgPane.setLayout(new BorderLayout());
      panel1.setLayout(new BorderLayout());
      panel1.setOpaque(false);//!!
      top.setBorder(BorderFactory.createTitledBorder(""));
      bottom.setBorder(BorderFactory.createTitledBorder(""));
      tf = new JTextField(1);
      JLabel img = new JLabel(ic, JLabel.CENTER);
      JLabel jl = new JLabel("Enter a letter", JLabel.CENTER);
      jlLetsUsed = new JLabel("Letters used:  ", JLabel.CENTER);
      final JLabel jlLines = new JLabel("__ ", JLabel.CENTER);
      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 ));
      imgPane.add(img);//center
      top.add(jl);//top center
      top.add(tf);//top center
      bottomLeft.add(jlLetsUsed);//bottom left position
      bottomRight.add(jlLines);//bottom right position
      bottom.add(bottomLeft);//bottom
      bottom.add(bottomRight);//bottom
      panel1.add(imgPane, BorderLayout.CENTER);//background image (center)
      panel1.add(top, BorderLayout.NORTH);//text field and jlabel (top)
      panel1.add(bottom, BorderLayout.SOUTH);// blank spaces and letters used (bottom)
      gameFrame.setJMenuBar(menuBar);
      gameFrame.setTitle("Hangman");
      gameFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
      gameFrame.setIconImage(
      new ImageIcon("hangmanIcon.png").getImage());
      gameFrame.setResizable(false);
      gameFrame.add(panel1);
      gameFrame.setSize(800, 500);
      gameFrame.setLocationRelativeTo(null);
      gameFrame.setVisible(true);

          int j = 0;
          String line = "";
          for(j = 0; j<64; j++) {
              wordLength[j] = wordList[j].length();//gets length of words in wordList
          }//end for

          int f = 2;//change num to change level
          int m = 0;
        //creates line first then put into .setText
          while(m<wordLength[f]) {
              line += "__ ";
              m++;
          }//end for
          jlLines.setText(line);

          tf.addActionListener(new ActionListener() {
              int wrong = 0;
          @Override
          public void actionPerformed(ActionEvent e) {//when enter key pressed
              JTextField tf = (JTextField)e.getSource();
             letter = tf.getText();
              jlLetsUsed.setText(jlLetsUsed.getText() + letter + " ");//sets jlabel text to users entered letter
              char[] jlabelText = jlLines.getText().toCharArray();//converts string to character array (array length is length of string)
              char userEnteredChar = letter.charAt(0);
              //int wrong = 0;
              int level = 2;//change num to change level
              if (!wordList[level].contains(letter)) {
                  wrong++;
                  if (wrong >= 6) {
                      System.out.println("He's dead, game over.");//prompt user
                  }
                  return;
              }
              int i = 0;
              for(i = 0; i<wordList[level].length(); i++){
                  if(wordList[level].charAt(i) == userEnteredChar){
                      jlabelText[3 * i] = ' ';
                      jlabelText[3 * i + 1] = userEnteredChar;
                      jlLines.setText(String.valueOf(jlabelText));
                  }
              }//end for
              }//end actionPerformed method
          });
      }//end window method
   }

public class GameMain {
   public static void main(String[] args) {
      GameStructure game = new GameStructure();
      game.window();
   }
}
share|improve this question

2 Answers 2

up vote 7 down vote accepted

You currently aren't following Java package naming standards.

package com.Game;

You need to keep a "tab" (bah dum tish) on your indenting. Right now it is somewhat hard to read your code because of that.


You don't clear the text field once a letter is entered. This means the user had to clear out the field themselves and then enter a new letter. We can make the user's life a lot easier.

tf.setText("");
tf.requestFocus();

Every time I play, the game uses the same word that it wants me to guess. Let's add some randomness.

Random rand = new Random();
int level = rand.nextInt(Arrays.asList(wordList).size() + 1);

That last point I made leads me to this point. Your code isn't very object oriented. I would recommend using an ArrayList instead of a String[] as a start.

private ArrayList<String> wordList = new ArrayList<String>(Arrays.asList("computer", "java", "activity", "alaska", "appearance", "article", "automobile", "basket", "birthday", "canada", "central", "character", "chicken", "chosen", "cutting", "daily", "darkness", "diagram", "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"));

I'll leave this up to you to include in your code.


Final code:

import java.awt.BorderLayout;
import java.awt.Font;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;
import java.util.Arrays;
import java.util.Random;

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.JPanel;
import javax.swing.JTextField;

class GameStructure
{
    private String[] wordList = { "computer", "java", "activity", "alaska", "appearance", "article", "automobile", "basket", "birthday", "canada", "central", "character", "chicken", "chosen", "cutting", "daily", "darkness", "diagram", "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];

    public void window()
    {
        JMenuBar menuBar = new JMenuBar();
        JMenu menu = new JMenu("File");
        menu.setMnemonic(KeyEvent.VK_A);
        menuBar.add(menu);
        JMenuItem menuItem = new JMenuItem("Developer", KeyEvent.VK_T);
        menu.add(menuItem);
        JMenuItem menuItem2 = new JMenuItem("Instructions", KeyEvent.VK_T);
        menu.add(menuItem2);
        JMenuItem menuItem3 = new JMenuItem("Levels", KeyEvent.VK_T);
        menu.add(menuItem3);
        JMenuItem menuItem4 = new JMenuItem("Restart", KeyEvent.VK_T);
        menu.add(menuItem4);
        JMenuItem menuItem5 = new JMenuItem("Exit", KeyEvent.VK_T);
        menu.add(menuItem5);

        ImageIcon ic = new ImageIcon("hangman2.png");
        JFrame gameFrame = new JFrame();
        JPanel bottomRight = new JPanel();
        JPanel bottomLeft = new JPanel();
        JPanel top = new JPanel();
        JPanel bottom = new JPanel();
        JPanel imgPane = new JPanel();
        JPanel panel1 = new JPanel();
        bottom.setLayout(new BoxLayout(bottom, BoxLayout.X_AXIS));
        imgPane.setLayout(new BorderLayout());
        panel1.setLayout(new BorderLayout());
        panel1.setOpaque(false);// !!
        top.setBorder(BorderFactory.createTitledBorder(""));
        bottom.setBorder(BorderFactory.createTitledBorder(""));
        tf = new JTextField(1);
        JLabel img = new JLabel(ic, JLabel.CENTER);
        JLabel jl = new JLabel("Enter a letter", JLabel.CENTER);
        jlLetsUsed = new JLabel("Letters used:  ", JLabel.CENTER);
        final JLabel jlLines = new JLabel("__ ", JLabel.CENTER);
        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));
        imgPane.add(img);// center
        top.add(jl);// top center
        top.add(tf);// top center
        bottomLeft.add(jlLetsUsed);// bottom left position
        bottomRight.add(jlLines);// bottom right position
        bottom.add(bottomLeft);// bottom
        bottom.add(bottomRight);// bottom
        panel1.add(imgPane, BorderLayout.CENTER);// background image (center)
        panel1.add(top, BorderLayout.NORTH);// text field and jlabel (top)
        panel1.add(bottom, BorderLayout.SOUTH);// blank spaces and letters used (bottom)
        gameFrame.setJMenuBar(menuBar);
        gameFrame.setTitle("Hangman");
        gameFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        gameFrame.setIconImage(new ImageIcon("hangmanIcon.png").getImage());
        gameFrame.setResizable(false);
        gameFrame.add(panel1);
        gameFrame.setSize(800, 500);
        gameFrame.setLocationRelativeTo(null);
        gameFrame.setVisible(true);

        int j = 0;
        String line = "";
        for (j = 0; j < 64; j++)
        {
            wordLength[j] = wordList[j].length();
        }

        int f = 2;
        int m = 0;
        while (m < wordLength[f])
        {
            line += "__ ";
            m++;
        }
        jlLines.setText(line);

        tf.addActionListener(new ActionListener()
        {
            int wrong = 0;

            @Override
            public void actionPerformed(ActionEvent e)
            {
                JTextField tf = (JTextField) e.getSource();
                letter = tf.getText();
                tf.setText("");
                tf.requestFocus();
                jlLetsUsed.setText(jlLetsUsed.getText() + letter + " ");
                char[] jlabelText = jlLines.getText().toCharArray();
                char userEnteredChar = letter.charAt(0);
                Random rand = new Random();
                int level = rand.nextInt(Arrays.asList(wordList).size() + 1);
                if (!wordList[level].contains(letter))
                {
                    wrong++;
                    if (wrong >= 6)
                    {
                        System.out.println("He's dead, game over.");
                    }
                    return;
                }
                int i = 0;
                for (i = 0; i < wordList[level].length(); i++)
                {
                    if (wordList[level].charAt(i) == userEnteredChar)
                    {
                        jlabelText[3 * i] = ' ';
                        jlabelText[3 * i + 1] = userEnteredChar;
                        jlLines.setText(String.valueOf(jlabelText));
                    }
                }
            }
        });
    }
}

public class Test
{
    public static void main(String[] args)
    {
        GameStructure game = new GameStructure();
        game.window();
    }
}
share|improve this answer
    
I know I'm not supposed to say thanks in comments but thanks, you really helped me out a lot with the finishing touches of the game. If it's not too much for me to ask, can I get your email so if I have any questions I can reach you? I'm 15 and just starting off in java and I'm really looking to advance in programming. Sorry about the whole life story. –  Anon Dec 25 '13 at 1:29
    
Is it better to have braces on their own line? –  Anon Dec 25 '13 at 1:51
    
What IDE do you use? –  Anon Dec 25 '13 at 1:55
1  
@Anon I use Eclipse. –  syb0rg Dec 25 '13 at 1:55
1  
IDE with auto-formatting is king. removes the need to waste time being OCD on ancillary parts. –  WernerCD Dec 25 '13 at 5:20

Some comments:

  • Make sure you have a consistent indentation style. It makes it difficult to follow the code flow, if there are multiple lines with different indentation.

  • You should separate the view and the game logic. Having both the Swing design setup and the actual game logic mixed up—in a single method—makes the code very hard to navigate. Ideally, both should be in separate classes.

  • Consider making a subclass of JFrame for your game view. It will be the only thing responsible for setting up the visuals. You can pass it a GameStructure object (which would be only the game logic), so it can combine everything.

  • for(j = 0; j<64; j++) — Instead of hard-coding the length of the worldList, you should be able to adjust automatically if I add new words to the word list without changing anything else.

  • tf.addActionListener(new ActionListener() { … — Instead of having an anonymous listener, consider making an actual type to clean everything up. As you only need to have a single listener, you can also make the class you’re in implement ActionListener, so you can pass this to addActionListener.

share|improve this answer
1  
Why the downvote? –  poke Dec 25 '13 at 2:10

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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