patternjavaMinor
Multiple random falling objects animation in Java
Viewed 0 times
randomanimationobjectsfallingjavamultiple
Problem
I am new to programming but I tried to make my code as readable as possible! Hopefully you can see what it does before you run the program. I did not implement a ups or fps controller, the animation may be fast for some computers and slower for others. I also tried to use the information provided to me from my first code review. Eventually I will be making this concept into a dodge the meteors type game.
Game class:
```
import javax.swing.*;
import java.awt.*;
public class Game extends JPanel {
//changing these values will change the size of the game, while still remaining functional
//within the size limit specified.
public static final int WINDOW_WIDTH = 600;
public static final int WINDOW_HEIGHT = 400;
//Creates a Square object Array
Square[] squareArray = new Square[20];
public Game() {
//initializes square objects
for (int i = 0; i < squareArray.length; i++)
squareArray[i] = new Square();
}
public void paint(Graphics graphics) {
//makes background black
graphics.setColor(Color.black);
graphics.fillRect(0, 0, WINDOW_WIDTH, WINDOW_HEIGHT);
//paints square objects to the screen
for (Square aSquareArray : squareArray) {
aSquareArray.paint(graphics);
}
}
public void update() {
//calls the Square class update method on the square objects
for (Square aSquareArray : squareArray) aSquareArray.update();
}
public static void main(String[] args) throws InterruptedException {
Game game = new Game();
JFrame frame = new JFrame();
frame.add(game);
frame.setVisible(true);
frame.setSize(WINDOW_WIDTH, WINDOW_HEIGHT);
frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
frame.setTitle("Raining Multiple Squares");
frame.setResizable(false);
frame.setLocationRelativeTo(null);
while (true) {
game.update();
Game class:
```
import javax.swing.*;
import java.awt.*;
public class Game extends JPanel {
//changing these values will change the size of the game, while still remaining functional
//within the size limit specified.
public static final int WINDOW_WIDTH = 600;
public static final int WINDOW_HEIGHT = 400;
//Creates a Square object Array
Square[] squareArray = new Square[20];
public Game() {
//initializes square objects
for (int i = 0; i < squareArray.length; i++)
squareArray[i] = new Square();
}
public void paint(Graphics graphics) {
//makes background black
graphics.setColor(Color.black);
graphics.fillRect(0, 0, WINDOW_WIDTH, WINDOW_HEIGHT);
//paints square objects to the screen
for (Square aSquareArray : squareArray) {
aSquareArray.paint(graphics);
}
}
public void update() {
//calls the Square class update method on the square objects
for (Square aSquareArray : squareArray) aSquareArray.update();
}
public static void main(String[] args) throws InterruptedException {
Game game = new Game();
JFrame frame = new JFrame();
frame.add(game);
frame.setVisible(true);
frame.setSize(WINDOW_WIDTH, WINDOW_HEIGHT);
frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
frame.setTitle("Raining Multiple Squares");
frame.setResizable(false);
frame.setLocationRelativeTo(null);
while (true) {
game.update();
Solution
Well done, you're making good progress.
Structural changes:
You should not be reusing the squares. Reusing objects is more efficient, and is great for things like particle systems (where tons of particles are created and destroyed every second). But your program is nowhere near resource intensive enough that reusing objects is necessary.
Reusing objects is unclear. It's not always easy to reset an object to it's original state and is not always clear when an object is reset.
You should create a new square whenever you need one, and remove them from your list when they reach the bottom.
Below is a review of the code as it currently is. Some of it will be obsolete when you stop reusing squares, so keep that in mind.
The game class:
Be careful with importing .* you're importing a lot of stuff you don't need, this makes reading the code harder.
When reading code you're always wondering about the intent of the author. "Did they really mean to use a string there", and other questions like them. So as an author, you have to try your hardest to be clear about your intent. This involves not importing everything, but only the things you need.
As a side note, I talk a lot about the reader, and other people reading your code, which you might not be too worried about if you're working alone. In that case please remember that you will have to read your code as well. If you come back to your code in six months, it will be like you've never seen it before. So please make it easy on yourself, and show your intent as clearly as you can.
Game is an ok name, but you could be a bit more precise, what kind of game is it?
While arrays are the most efficient, in this case I think it's better to use a list instead. Using a list will make it much easier to add and remove squares.
The 20 you use in the array can be a constant (TOTAL_AMOUNT_OF_SQUARES).
I noticed that every method in this class starts with an empty line. This is not necessary and looks a bit sloppy.
This comment is not really nessesary. The code it describes is self explanatory. It can create some annoying mistakes, because it is easy to forget to change the comment when you change the code. Here is some nice discussion on when to use comments. Be sure to look around the related questions as well.
The background can be made into a constant as well (BACKGROUND_COLOR).
The name of the square in the loop does not have to be aSquareArray, but can simply be square. so the line can read "for (Square square: squareArray)".
Always use brackets for loops. I know, I know, they take up a lot of space, but if you don't use them, you will run into some nasty problems. (to bracket or not to bracket, has by this point turned into somewhat of a holy war, but especially for beginners I recommend using them, because even though they take up space, they promote clarity).
This whole frame setup is pretty long, you could consider moving it to it's own method, just to keep it out of the way.
The Square class:
Square does not have to extend JPanel, and actually it shouldn't. Because it's not a JPanel.
```
private int squareXLocation;
private int squareSize;
private int squareYLocation = -squareSi
Structural changes:
You should not be reusing the squares. Reusing objects is more efficient, and is great for things like particle systems (where tons of particles are created and destroyed every second). But your program is nowhere near resource intensive enough that reusing objects is necessary.
Reusing objects is unclear. It's not always easy to reset an object to it's original state and is not always clear when an object is reset.
You should create a new square whenever you need one, and remove them from your list when they reach the bottom.
Below is a review of the code as it currently is. Some of it will be obsolete when you stop reusing squares, so keep that in mind.
The game class:
import javax.swing.*;
import java.awt.*;Be careful with importing .* you're importing a lot of stuff you don't need, this makes reading the code harder.
When reading code you're always wondering about the intent of the author. "Did they really mean to use a string there", and other questions like them. So as an author, you have to try your hardest to be clear about your intent. This involves not importing everything, but only the things you need.
As a side note, I talk a lot about the reader, and other people reading your code, which you might not be too worried about if you're working alone. In that case please remember that you will have to read your code as well. If you come back to your code in six months, it will be like you've never seen it before. So please make it easy on yourself, and show your intent as clearly as you can.
public class Game extends JPanel {Game is an ok name, but you could be a bit more precise, what kind of game is it?
//changing these values will change the size of the game, while still remaining functional
//within the size limit specified.
public static final int WINDOW_WIDTH = 600;
public static final int WINDOW_HEIGHT = 400;
//Creates a Square object Array
Square[] squareArray = new Square[20];While arrays are the most efficient, in this case I think it's better to use a list instead. Using a list will make it much easier to add and remove squares.
The 20 you use in the array can be a constant (TOTAL_AMOUNT_OF_SQUARES).
public Game() {
//initializes square objects
for (int i = 0; i < squareArray.length; i++)
squareArray[i] = new Square();
}I noticed that every method in this class starts with an empty line. This is not necessary and looks a bit sloppy.
public void paint(Graphics graphics) {
//makes background blackThis comment is not really nessesary. The code it describes is self explanatory. It can create some annoying mistakes, because it is easy to forget to change the comment when you change the code. Here is some nice discussion on when to use comments. Be sure to look around the related questions as well.
graphics.setColor(Color.black);The background can be made into a constant as well (BACKGROUND_COLOR).
graphics.fillRect(0, 0, WINDOW_WIDTH, WINDOW_HEIGHT);
//paints square objects to the screen
for (Square aSquareArray : squareArray) {
aSquareArray.paint(graphics);
}The name of the square in the loop does not have to be aSquareArray, but can simply be square. so the line can read "for (Square square: squareArray)".
}
public void update() {
//calls the Square class update method on the square objects
for (Square aSquareArray : squareArray) aSquareArray.update();
}Always use brackets for loops. I know, I know, they take up a lot of space, but if you don't use them, you will run into some nasty problems. (to bracket or not to bracket, has by this point turned into somewhat of a holy war, but especially for beginners I recommend using them, because even though they take up space, they promote clarity).
public static void main(String[] args) throws InterruptedException {
Game game = new Game();
JFrame frame = new JFrame();
frame.add(game);
frame.setVisible(true);
frame.setSize(WINDOW_WIDTH, WINDOW_HEIGHT);
frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
frame.setTitle("Raining Multiple Squares");
frame.setResizable(false);
frame.setLocationRelativeTo(null);This whole frame setup is pretty long, you could consider moving it to it's own method, just to keep it out of the way.
while (true) {
game.update();
game.repaint();
Thread.sleep(10);
}
}
}The Square class:
import javax.swing.*;
import java.awt.*;
import java.util.Random;
public class Square extends JPanel {Square does not have to extend JPanel, and actually it shouldn't. Because it's not a JPanel.
```
private int squareXLocation;
private int squareSize;
private int squareYLocation = -squareSi
Code Snippets
import javax.swing.*;
import java.awt.*;public class Game extends JPanel {//changing these values will change the size of the game, while still remaining functional
//within the size limit specified.
public static final int WINDOW_WIDTH = 600;
public static final int WINDOW_HEIGHT = 400;
//Creates a Square object Array
Square[] squareArray = new Square[20];public Game() {
//initializes square objects
for (int i = 0; i < squareArray.length; i++)
squareArray[i] = new Square();
}public void paint(Graphics graphics) {
//makes background blackContext
StackExchange Code Review Q#73959, answer score: 4
Revisions (0)
No revisions yet.