patternjavaMinor
Reorganize a maze game from one large class into smaller classes
Viewed 0 times
mazeintoonegamelargesmallerclassesfromclassreorganize
Problem
I'm new to Java and I really don't understand how to make more classes work like one. So if anybody could explain and help me divide my large code into smaller classes (like Player.java, Main.java, Move.java, etc.). I want to check if the player hits the wall, and the easiest way is to make player object a blocks object and then check if they intersect. But I don't know how to make objects from what I have.
```
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Toolkit;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.awt.image.BufferedImage;
import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.io.InputStreamReader;
import javax.swing.Timer;
import javax.imageio.ImageIO;
import javax.swing.JFrame;
import org.omg.CORBA.portable.InputStream;
import java.io.Reader;
public class NewTest extends JFrame implements KeyListener {
/**
*
*/
private static final long serialVersionUID = 1L;
private Image TestImage;
private BufferedImage bf;
private BufferedImage bufferedImage;
private int cordX = 100;
private int cordY = 100;
public int mapy=25;
public int mapx=mapy;
public int size= 20;
private boolean down, up, left, right;
private Image wall = Toolkit.getDefaultToolkit().getImage("image/Koala.jpg");
private Image no = Toolkit.getDefaultToolkit().getImage("image/house.jpg");
public static int[][]
map = {{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
{0, 10, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 11},
{0, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3},
{0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
```
import java.awt.Color;
import java.awt.Graphics;
import java.awt.Graphics2D;
import java.awt.Image;
import java.awt.Toolkit;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.awt.image.BufferedImage;
import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.io.InputStreamReader;
import javax.swing.Timer;
import javax.imageio.ImageIO;
import javax.swing.JFrame;
import org.omg.CORBA.portable.InputStream;
import java.io.Reader;
public class NewTest extends JFrame implements KeyListener {
/**
*
*/
private static final long serialVersionUID = 1L;
private Image TestImage;
private BufferedImage bf;
private BufferedImage bufferedImage;
private int cordX = 100;
private int cordY = 100;
public int mapy=25;
public int mapx=mapy;
public int size= 20;
private boolean down, up, left, right;
private Image wall = Toolkit.getDefaultToolkit().getImage("image/Koala.jpg");
private Image no = Toolkit.getDefaultToolkit().getImage("image/house.jpg");
public static int[][]
map = {{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
{0, 10, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 11},
{0, 2, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3},
{0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
Solution
Splitting your classes
Rename
As a first step, the main class should not implement
Once your class has a proper name corresponding to its main responsibility, the
Keep looking at the main class that seems to do too many things with a critical eye, keeping in mind the single responsibility principle. Check every feature if's part of the main responsibility or if it can be extracted to a class of its own. Repeat until you cannot find any more features to extract, when everything seem to fit well, and the class looks like a good cohesive unit.
The single responsibility principle
Apply the single responsibility principle to methods even more strictly than classes. For example this is strange:
This method loads an image, and then... adds a listener to this? That second step doesn't make any sense. And the first step was actually really 2 steps: load an image and assign it to a field. So that already violated the single responsibility principle.
This method would be better this way:
Now it does only one thing: it loads an image. Even better, it can load any image specified by a path. Even better, it doesn't mess with the internal state of the main class. And at this point I hope you see that this method could be a good candidate to extract to a utility class.
Bugs
The
If
The same goes for
Exception handling
This seems pointless:
Without the try-catch, effectively the same thing will happen: the program will crash, and the stack trace will be printed on the console. Even better, its exit code will be a failure, instead of success. Just let it crash:
Rename
NewTest to something more meaningful. What is the main responsibility of this class? Think about that, and rename it accordingly.As a first step, the main class should not implement
KeyListener. A class should have only one responsibility, and a main class already has one: being the main class. Create an inner class that implements KeyListener and trigger the appropriate state updates.Once your class has a proper name corresponding to its main responsibility, the
main method should start to look odd... You see, launching the class is yet another responsibility. So move the launching functionality out to a different class, for example KoalaGameRunner. This might seem a bit overkill at first, but it's not. You can envision in the future adding command line parsing functionality in KoalaGameRunner.Keep looking at the main class that seems to do too many things with a critical eye, keeping in mind the single responsibility principle. Check every feature if's part of the main responsibility or if it can be extracted to a class of its own. Repeat until you cannot find any more features to extract, when everything seem to fit well, and the class looks like a good cohesive unit.
The single responsibility principle
Apply the single responsibility principle to methods even more strictly than classes. For example this is strange:
public void imageLoader() throws IOException {
TestImage = Toolkit.getDefaultToolkit().getImage("image/Koala.jpg");
addKeyListener(this);
}This method loads an image, and then... adds a listener to this? That second step doesn't make any sense. And the first step was actually really 2 steps: load an image and assign it to a field. So that already violated the single responsibility principle.
This method would be better this way:
Image imageLoader(String path) throws IOException {
return Toolkit.getDefaultToolkit().getImage(path);
}Now it does only one thing: it loads an image. Even better, it can load any image specified by a path. Even better, it doesn't mess with the internal state of the main class. And at this point I hope you see that this method could be a good candidate to extract to a utility class.
Bugs
The
updateState method seems to have a bug in the logic:if (right) {
cordX += 5;
} else if (left) {
cordX -= 5;
}
if (right && left) {
cordX += 0;
cordY += 0;
}If
right and left are pressed at the same time, then you increment cordX by 5, and then you have the right && left condition, to do nothing. You probably meant this way:if (right && left) {
// do nothing
} else if (right) {
cordX += 5;
} else if (left) {
cordX -= 5;
}The same goes for
up and down.Exception handling
This seems pointless:
public static void main(String[] args) {
try {
new NewTest();
} catch (IOException e) {
e.printStackTrace();
}
}Without the try-catch, effectively the same thing will happen: the program will crash, and the stack trace will be printed on the console. Even better, its exit code will be a failure, instead of success. Just let it crash:
public static void main(String[] args) throws IOException {
new NewTest();
}Code Snippets
public void imageLoader() throws IOException {
TestImage = Toolkit.getDefaultToolkit().getImage("image/Koala.jpg");
addKeyListener(this);
}Image imageLoader(String path) throws IOException {
return Toolkit.getDefaultToolkit().getImage(path);
}if (right) {
cordX += 5;
} else if (left) {
cordX -= 5;
}
if (right && left) {
cordX += 0;
cordY += 0;
}if (right && left) {
// do nothing
} else if (right) {
cordX += 5;
} else if (left) {
cordX -= 5;
}public static void main(String[] args) {
try {
new NewTest();
} catch (IOException e) {
e.printStackTrace();
}
}Context
StackExchange Code Review Q#61606, answer score: 4
Revisions (0)
No revisions yet.