6
\$\begingroup\$

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();
 }
}
Pimgd
22.5k5 gold badges68 silver badges144 bronze badges
asked Dec 24, 2013 at 22:18
\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

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();
 }
}
answered Dec 25, 2013 at 0:21
\$\endgroup\$
5
  • \$\begingroup\$ 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. \$\endgroup\$ Commented Dec 25, 2013 at 1:29
  • \$\begingroup\$ Is it better to have braces on their own line? \$\endgroup\$ Commented Dec 25, 2013 at 1:51
  • \$\begingroup\$ What IDE do you use? \$\endgroup\$ Commented Dec 25, 2013 at 1:55
  • 1
    \$\begingroup\$ IDE with auto-formatting is king. removes the need to waste time being OCD on ancillary parts. \$\endgroup\$ Commented Dec 25, 2013 at 5:20
  • \$\begingroup\$ Don't you mean int level = rand.nextInt(wordList.length + 1); ? \$\endgroup\$ Commented Feb 5, 2014 at 5:14
7
\$\begingroup\$

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.

syb0rg
21.9k10 gold badges113 silver badges192 bronze badges
answered Dec 24, 2013 at 22:30
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Why the downvote? \$\endgroup\$ Commented Dec 25, 2013 at 2:10

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.