I would like some help refactoring this code. This is to improve some of the nonfunctional attributes of the software to
- improve code readability
- 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();
}
}
2 Answers 2
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();
}
}
-
\$\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\$Anon– Anon2013年12月25日 01:29:06 +00:00Commented Dec 25, 2013 at 1:29
-
\$\begingroup\$ Is it better to have braces on their own line? \$\endgroup\$Anon– Anon2013年12月25日 01:51:18 +00:00Commented Dec 25, 2013 at 1:51
-
\$\begingroup\$ What IDE do you use? \$\endgroup\$Anon– Anon2013年12月25日 01:55:05 +00:00Commented 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\$WernerCD– WernerCD2013年12月25日 05:20:26 +00:00Commented Dec 25, 2013 at 5:20
-
\$\begingroup\$ Don't you mean
int level = rand.nextInt(wordList.length + 1);
? \$\endgroup\$200_success– 200_success2014年02月05日 05:14:37 +00:00Commented Feb 5, 2014 at 5:14
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 aGameStructure
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 theworldList
, 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 implementActionListener
, so you can passthis
toaddActionListener
.
-
1\$\begingroup\$ Why the downvote? \$\endgroup\$poke– poke2013年12月25日 02:10:15 +00:00Commented Dec 25, 2013 at 2:10
Explore related questions
See similar questions with these tags.