Space Invaders clone for a college project

Question

Edit: the project was submitted, and I got a good grade! :D

I am finishing up a project for my computer science class. The project is a space invaders clone that uses the JavaFX library to provide graphics. The program itself is complete and functional, but I wanted to make sure that the code itself is readable and proper. The due date is tomorrow (December 12, 12 AM), so it would be great if I could get an answer before then, but if the date is passed I would still appreciate any advice on improving my code structure/practices (I'm a lurker, but I'm new to posting on codereview/StackExchange/StackOverflow, so please excuse me if this isn't proper form. I have no intention of being rude or demanding). I am specifically looking for criticism that covers:

• Modular design
• Documentation
• Efficiency
• Proper OOP
• Code Structure
• Anything else you notice that is poorly written/implemented

I have listed my code below, separated by class. If you put all code segments into a file in the order in which they were presented, it would form the source code.

MyProj class/imported references:

package myproj;
import java.util.ArrayList;
import javafx.scene.shape.Polygon;
import javafx.scene.shape.Polyline;
import javafx.scene.layout.Pane;
import javafx.scene.text.Text;
import javafx.scene.text.TextAlignment;
import javafx.scene.Scene;
import javafx.scene.input.KeyCode;
import javafx.scene.paint.Color;
import javafx.event.ActionEvent;
import javafx.scene.Node;
import javafx.stage.Stage;
import javafx.event.EventHandler;
import javafx.animation.KeyFrame;
import javafx.animation.Timeline;
import javafx.util.Duration;
import javafx.application.Application;
public class MyProj extends Application{
 public void start(Stage mainStage){
 GameManager gameManager = new GameManager();
 Scene mainScene = new Scene(gameManager.gamePane, 500, 600);
 mainStage.setScene(mainScene);
 mainStage.setResizable(false);
 mainStage.show();
 mainScene.setFill(Color.BLACK);
 
 //Set the game pane to the start screen
 gameManager.gamePane.requestFocus();
 gameManager.startScreen("Space Invaders\n\nPress arrow keys to move\nPress space to shoot\nKill all of the aliens to win\n\nPress any key to start game");
 }
 
 public static void main(String[] args) {
 Application.launch(args);
 }
}

GameManager class:

//GameManager serves as the manager of the pane that contains the game and
//everything in it. It handles collisions, movement, controls, score, lives, setting up
//the game, and starting the game.
class GameManager{
 Pane gamePane;
 Player player;
 AlienGridController alienGrid;
 Bullet activePlayerBullet;
 
 Text scoreText;
 Text lifeText;
 private int score;
 private int lives;
 
 //Increments in pixels that the player and alien grid move in
 private int playerMovementIncrement = 5;
 private int alienGridMovementIncrementX = 10;
 private int alienGridMovementIncrementY = 20;
 
 //Event handler that moves the player
 EventHandler<ActionEvent> playerMovement = e -> {
 //Checks to see if the player will be out of bounds if they move an
 //increment, and move them if they won't be
 if(player.getTranslateX() + player.getBoundsInLocal().getWidth() + playerMovementIncrement < gamePane.getWidth() && player.getTranslateX() + playerMovementIncrement > 0)
 player.setTranslateX(player.getTranslateX() + playerMovementIncrement);
 };
 
 //Event handler that moves the alien grid
 EventHandler<ActionEvent> alienGridMovement = e -> {
 //Ensures that the column and row borders are calibrated in order for
 //getWidth() and getHeight() to be accurate
 alienGrid.calibrateColumnBorders();
 alienGrid.calibrateRowBorders();
 
 //Check if the alien grid will be out of bounds if it moves another
 //increment. If it will be, then it is moved down and the x movement 
 //increment is inverted. If the alien grid won't be out of bounds,
 //then move it an x increment.
 //In the case that the alien grid manages to get to the level of the
 //player, then invoke startScreen()
 if(alienGrid.getTranslateX() + alienGrid.getWidth() + alienGridMovementIncrementX > gamePane.getWidth() || alienGrid.getTranslateX() + alienGridMovementIncrementX < 0){
 alienGrid.moveY(alienGridMovementIncrementY);
 alienGridMovementIncrementX = Math.negateExact(alienGridMovementIncrementX);
 if(alienGrid.getHeight() + alienGrid.getTranslateY() + alienGridMovementIncrementY > player.getTranslateY())
 startScreen("Game Over\n\npress any key to restart");
 }
 
 else
 alienGrid.moveX(alienGridMovementIncrementX);
 };
 
 //Event handler that detects if bullets hit the player, aliens, or walls,
 //and acts accordingly
 EventHandler<ActionEvent> bulletCollisionDetection = e -> {
 //Gets the list of bullets currently in gamePane
 ArrayList<Node> bulletList = getBullets();
 
 //Iterate through the bullets
 for(int i = 0; i < bulletList.size(); i++){
 Bullet bullet = (Bullet)bulletList.get(i);
 
 //Check if bullet is out of bounds, and remove accordingly
 if(bullet.getTranslateY() > gamePane.getHeight() || bullet.getTranslateY() < 0){
 gamePane.getChildren().remove(bullet);
 }
 //Check if bullet has collided with the player. If the bullet tag is
 //Bullet.ALIEN, then remove the bullet and invoke playerDeath()
 else if(player.getBoundsInParent().intersects(bullet.getBoundsInParent()) && bullet.tag.equals(Bullet.ALIEN)){
 gamePane.getChildren().remove(bullet);
 playerDeath();
 }
 
 //Check if the bullet has collided with an alien. If it has, then
 //remove the bullet and alien, set the corresponding alien grid cell
 //to null, add 100 points to the score, and check if the grid is
 //empty.
 else{ 
 for(int column = 0; column < alienGrid.getAlienList().length; column++){
 for(int row = 0; row < alienGrid.getAlienList()[column].length; row++){
 if(alienGrid.getAlienList()[column][row] != null){
 if(alienGrid.getAlienList()[column][row].getBoundsInParent().intersects(bullet.getBoundsInParent())){
 gamePane.getChildren().remove(bullet);
 gamePane.getChildren().remove(alienGrid.getAlienList()[column][row]);
 alienGrid.getAlienList()[column][row] = null;
 setScore(score + 100);
 if(alienGrid.isEmpty())
 startScreen("You Won!\n\nPress any key to restart");
 }
 }
 }
 }
 }
 }
 };
 
 //Event handler that fires bullets from alien grid
 EventHandler<ActionEvent> alienFire = e -> {
 int randomColumn;
 Alien lowestAlien;
 while(true){
 //Choose random column and check if it's empty. if so, then choose
 //another.
 randomColumn = (int)((alienGrid.getRightBorder() - alienGrid.getLeftBorder()) * Math.random()) + alienGrid.getLeftBorder();
 if(alienGrid.getAlienList()[randomColumn] != null){
 break;
 }
 }
 
 for(int row = alienGrid.getAlienList()[randomColumn].length - 1; row > 0; row--){
 //Iterate through the rows for the column, starting from the lowest.
 //If the current row contains an alien, fire a bullet from that alien.
 if(alienGrid.getAlienList()[randomColumn][row] != null){
 lowestAlien = alienGrid.getAlienList()[randomColumn][row];
 gamePane.getChildren().add(new Bullet(lowestAlien.getTranslateX() + lowestAlien.getBoundsInLocal().getWidth() / 2, lowestAlien.getTranslateY() + lowestAlien.getBoundsInLocal().getHeight(), -3, Bullet.ALIEN));
 break;
 }
 }
 };
 
 //Timelines for the four previous event handlers
 Timeline playerMovementTimeline = new Timeline(new KeyFrame(Duration.millis(20), playerMovement));
 Timeline alienGridMovementTimeline = new Timeline(new KeyFrame(Duration.millis(500), alienGridMovement));
 Timeline bulletCollisionDetectionTimeline = new Timeline(new KeyFrame(Duration.millis(50), bulletCollisionDetection));
 Timeline alienFireTimeline = new Timeline(new KeyFrame(Duration.seconds(1), alienFire));
 
 //Create new pane, score text, and life text objects, and set cycle counts
 //for the Timeline objects to indefinite.
 GameManager(){
 gamePane = new Pane();
 scoreText = new Text();
 lifeText = new Text();
 playerMovementTimeline.setCycleCount(Timeline.INDEFINITE);
 alienGridMovementTimeline.setCycleCount(Timeline.INDEFINITE);
 bulletCollisionDetectionTimeline.setCycleCount(Timeline.INDEFINITE);
 alienFireTimeline.setCycleCount(Timeline.INDEFINITE);
 }
 
 //Set up gamePane and its children
 void setUpGame(){
 //Clear the game pane of all nodes
 gamePane.getChildren().clear();
 
 this.player = new Player();
 this.alienGrid = new AlienGridController(5, 8);
 
 //Set the score to 0 and position/color the score text
 setScore(0);
 scoreText.setTranslateX(10);
 scoreText.setTranslateY(10);
 scoreText.setStroke(Color.WHITE);
 
 //Set the lives to 3 and position/color the life text
 setLives(3);
 lifeText.setTranslateX(gamePane.getWidth() - lifeText.getBoundsInLocal().getWidth() - 10);
 lifeText.setTranslateY(10);
 lifeText.setStroke(Color.WHITE);
 
 //Position the alien grid
 alienGrid.setTranslateX((gamePane.getWidth() / 2) - (alienGrid.getWidth() / 2));
 alienGrid.setTranslateY(50);
 
 //Position the player
 player.setTranslateX((gamePane.getWidth() / 2) - (player.getBoundsInLocal().getWidth() / 2));
 player.setTranslateY(gamePane.getHeight() - 50);
 
 //Add all of the aliens in the alien grid to the gamePane
 gamePane.getChildren().addAll(player, scoreText, lifeText);
 for(int column = 0; column < alienGrid.getAlienList().length; column++)
 for(int row = 0; row < alienGrid.getAlienList()[column].length; row++)
 gamePane.getChildren().add(alienGrid.getAlienList()[column][row]);
 }
 
 //Start the alien movement, bullet collision, and alien firing timelines,
 //and enable controls.
 void startGame(){
 alienGridMovementTimeline.play();
 bulletCollisionDetectionTimeline.play();
 alienFireTimeline.play();
 enableControls();
 }
 
 //Arranges the player's death, and ends game if necessary
 void playerDeath(){
 //Event handler that is played after the deathTimeline
 EventHandler<ActionEvent> deathTimelineFinished = e -> {
 //If no more lives remain, invoke startScreen
 if(lives <= 0)
 startScreen("Game Over\n\npress any key to restart");
 
 else{
 //Remove a life
 setLives(lives - 1);
 
 //Start necessary timelines
 alienGridMovementTimeline.play();
 alienFireTimeline.play();
 
 enableControls();
 
 //Center player
 player.setTranslateX(gamePane.getWidth() / 2 - (player.getBoundsInLocal().getWidth() / 2));
 player.setTranslateY(gamePane.getHeight() - 50);
 
 //In case of a freak one-off error on the timeline's part, 
 //ensure that player is visible
 player.setVisible(true);
 }
 };
 //Timeline that flashes player to show damage
 Timeline deathTimeline = new Timeline(new KeyFrame(Duration.millis(300), e -> player.setVisible(!player.isVisible())));
 
 deathTimeline.setOnFinished(deathTimelineFinished);
 deathTimeline.setCycleCount(4);
 //Pause necessary timelines
 alienGridMovementTimeline.pause();
 alienFireTimeline.pause();
 playerMovementTimeline.pause();
 
 disableControls();
 
 //Remove all bullets
 gamePane.getChildren().removeAll(getBullets());
 
 deathTimeline.play();
 }
 
 //Displays simple start screen that starts the game when any key is pressed 
 void startScreen(String message){
 //Clear the game pane of all nodes
 gamePane.getChildren().clear();
 
 
 Text startScreenText = new Text(message);
 
 //Move, style, and add the start screen text to the game pane
 startScreenText.setTextAlignment(TextAlignment.CENTER);
 startScreenText.setTranslateX((gamePane.getWidth() / 2) - (startScreenText.getBoundsInLocal().getWidth() / 2));
 startScreenText.setTranslateY((gamePane.getHeight() / 2) - (startScreenText.getBoundsInLocal().getHeight() / 2));
 startScreenText.setStroke(Color.WHITE);
 gamePane.getChildren().add(startScreenText);
 
 //Bind any key press to set up and start game
 gamePane.setOnKeyPressed(e -> {
 setUpGame();
 startGame();
 });
 }
 
 //Bind the player controls
 void enableControls(){
 gamePane.setOnKeyPressed(e -> {
 //Starts player movement to the left when left key is pressed
 if(e.getCode().equals(KeyCode.LEFT)){
 playerMovementTimeline.play();
 playerMovementIncrement = -2;
 }
 
 //Starts player movement to the right when right key is pressed
 if(e.getCode().equals(KeyCode.RIGHT)){
 playerMovementTimeline.play();
 playerMovementIncrement = 2;
 }
 
 //If there is not already a bullet launched by the player in the
 //game pane, then launch one when space is pressed
 if(e.getCode().equals(KeyCode.SPACE)){
 if(!gamePane.getChildren().contains(activePlayerBullet)){
 activePlayerBullet = new Bullet(player.getTranslateX() + player.getBoundsInLocal().getWidth() / 2, player.getTranslateY() - 20, 3, Bullet.PLAYER);
 gamePane.getChildren().add(activePlayerBullet);
 }
 }
 });
 
 gamePane.setOnKeyReleased(e -> {
 //Stop player movement if a player movement key is released
 if(e.getCode().equals(KeyCode.LEFT) || e.getCode().equals(KeyCode.RIGHT)){
 playerMovementTimeline.pause();
 }
 });
 }
 
 //Disables the player controls
 void disableControls(){
 //Bind key presses to null, disabling controls
 gamePane.setOnKeyPressed(null);
 gamePane.setOnKeyReleased(null);
 }
 
 //Gets all bullets currently in the game pane
 ArrayList<Node> getBullets(){
 ArrayList<Node> bulletList = new ArrayList<Node>();
 //Iterates through the nodes in game pane. If the current node is an 
 //instance of Bullet, then add it to the bullet list;
 for(int node = 0; node < gamePane.getChildren().size(); node++)
 if(gamePane.getChildren().get(node) instanceof Bullet)
 bulletList.add(gamePane.getChildren().get(node));
 
 return bulletList;
 }
 
 //Set score and scoreText
 void setScore(int score){
 this.score = score;
 scoreText.setText("Score: " + score);
 }
 
 //Set lives and lifeText
 void setLives(int lives){
 this.lives = lives;
 lifeText.setText("Lives: " + lives);
 }
}

AlienGridController Class:

//AlienGridController arranges alien objects in a grid pattern, moves and
//positions them in unision, and performs calculations to determine its borders,
//width, and height for the convenience of gameManager. I originally planned to 
//use gridPane, but a combination of factors(collision detection, alien placement, etc)
//led to me making my own class.
class AlienGridController{
 private Alien[][] alienList;
 private Alien outerAlienLeft;
 private Alien outerAlienRight;
 private Alien outerAlienTop;
 private Alien outerAlienBottom;
 private int leftBorder = 0;
 private int rightBorder = 0;
 private int topBorder = 0;
 private int bottomBorder = 0;
 private double cellMargin = 10;
 
 
 AlienGridController(int rows, int columns){
 //Initializes the alien list to hold the specified rows and columns
 alienList = new Alien[columns][rows];
 
 //Iterate through the alien list, setting each value to a new alien
 //object and positioning it according to previous entries and the cell
 //margin
 for(int column = 0; column < columns; column++){
 for(int row = 0; row < rows; row++){
 alienList[column][row] = new Alien();
 alienList[column][row].setTranslateX(row * (cellMargin + alienList[column][row].getBoundsInLocal().getWidth()));
 alienList[column][row].setTranslateY(row * (cellMargin + alienList[column][row].getBoundsInLocal().getHeight()));
 }
 }
 
 //Initially calibrate the column and row borders and set the translate
 //values to 0
 calibrateColumnBorders();
 calibrateRowBorders();
 setTranslateX(0);
 setTranslateY(0);
 }
 
 //Finds the far left and far right column index and alien objects in the alien
 //grid
 void calibrateColumnBorders(){
 boolean leftBorderSet = false;
 boolean rightBorderSet = false;
 
 //Iterate through the alien list, starting from the left. Once a alien
 //object is found, the left border is set as that iteration's column, 
 //and the alien object that is furthest left is set as that iteration's
 //alien.
 for(int column = 0; column < alienList.length; column++){
 for(int row = 0; row < alienList[column].length; row++){
 if(alienList[column][row] != null){
 leftBorder = column;
 outerAlienLeft = alienList[column][row];
 leftBorderSet = true;
 break;
 }
 }
 if(leftBorderSet)
 break;
 }
 
 //Iterate through the alien list, starting from the right. Once a alien
 //object is found, the right border is set as that iteration's column, 
 //and the alien object that is furthest right is set as that iteration's
 //alien.
 for(int column = alienList.length - 1; column >= 0; column--){
 for(int row = alienList[column].length - 1; row >= 0; row--){
 if(alienList[column][row] != null){
 rightBorder = column;
 outerAlienRight = alienList[column][row];
 rightBorderSet = true;
 break;
 }
 }
 if(rightBorderSet)
 break;
 }
 }
 
 //Finds the top and bottom row index and alien objects in the alien grid
 void calibrateRowBorders(){
 boolean topBorderSet = false;
 boolean bottomBorderSet = false;
 
 //Iterate through the alien list, starting from the top. Once a alien
 //object is found, the top border is set as that iteration's row, 
 //and the alien object that is furthest up is set as that iteration's
 //alien.
 for(int row = 0; row < alienList[0].length; row++){
 for(int column = 0; column < alienList.length; column++){
 if(alienList[column][row] != null){
 topBorder = row;
 outerAlienTop = alienList[column][row];
 topBorderSet = true;
 break;
 }
 }
 if(topBorderSet)
 break;
 }
 
 //Iterate through the alien list, starting from the bottom. Once a alien
 //object is found, the bottom border is set as that iteration's row, 
 //and the alien object that is furthest down is set as that iteration's
 //alien.
 for(int row = alienList[0].length - 1; row >= 0; row--){
 for(int column = alienList.length - 1; column >= 0; column--){
 if(alienList[column][row] != null){
 bottomBorder = row;
 outerAlienBottom = alienList[column][row];
 bottomBorderSet = true;
 break;
 }
 }
 if(bottomBorderSet)
 break;
 }
 }
 
 //Get the list of aliens in the alien grid
 public Alien[][] getAlienList(){
 return alienList;
 }
 
 //Check if the grid is empty
 public boolean isEmpty(){
 //Iterate through the alien grid. If an alien is found, return false
 for(int column = 0; column < alienList.length; column++)
 for(int row = 0; row < alienList[column].length; row++)
 if(alienList[column][row] != null)
 return false;
 
 return true;
 }
 
 //Set the X transform of the aliens inside the grid
 void setTranslateX(double position){
 //Iterate through the alien grid. If an alien is found, than transform it
 //while accounting for the cell margin and aliens to the lefr
 for(int column = 0; column < alienList.length; column++)
 for(int row = 0; row < alienList[column].length; row++)
 if(alienList[column][row] != null)
 alienList[column][row].setTranslateX(position + (column * (cellMargin + alienList[column][row].getBoundsInLocal().getWidth())));
 }
 
 //Set the Y transform of the aliens inside the grid
 void setTranslateY(double position){
 //Iterate through the alien grid. If an alien is found, than transform it
 //while accounting for the cell margin and aliens above
 for(int column = 0; column < alienList.length; column++)
 for(int row = 0; row < alienList[column].length; row++)
 if(alienList[column][row] != null)
 alienList[column][row].setTranslateY(position + (row * (cellMargin + alienList[column][row].getBoundsInLocal().getHeight())));
 }
 
 //Get the x position of the outer left alien, which represents the alien grid's
 //x position
 double getTranslateX(){
 return outerAlienLeft.getTranslateX();
 }
 
 //Get the y position of the outer top alien, which represents the alien grid's
 //y position
 double getTranslateY(){
 return outerAlienTop.getTranslateY();
 }
 
 //Increments the alien grid on the x axis
 void moveX(double increment){
 setTranslateX(getTranslateX() + increment);
 }
 
 //Increments the alien grid on the y axis
 void moveY(double increment){
 setTranslateY(getTranslateY() + increment);
 }
 
 //Get the width of the alien grid
 double getWidth(){
 return outerAlienRight.getTranslateX() + outerAlienRight.getBoundsInLocal().getWidth() - outerAlienLeft.getTranslateX();
 }
 
 //Get the height of the alien grid
 double getHeight(){
 return outerAlienBottom.getTranslateY() + outerAlienBottom.getBoundsInLocal().getHeight() - outerAlienTop.getTranslateY();
 }
 
 //Get the index of the right border
 int getRightBorder(){
 return rightBorder;
 }
 
 //Get the index of the left border
 int getLeftBorder(){
 return leftBorder;
 }
 
 //set the cell margin
 void setCellMargin(double cellMargin){
 this.cellMargin = cellMargin;
 }
 double getCellMargin(){
 return cellMargin;
 }
}

Alien class:

 //The Alien class constructs a Polygon node that can be manipulated
class Alien extends Polygon{
 Alien(){
 super(0.0, 4.0, 0.0, 12.0, 4.0, 16.0,
 8.0, 16.0, 6.0, 24.0, 10.0, 24.0,
 12.0, 16.0, 16.0, 16.0, 18.0, 24.0,
 22.0, 24.0, 20.0, 16.0, 24.0, 16.0,
 28.0, 12.0, 28.0, 4.0, 24.0, 0.0,
 4.0, 0.0);
 
 setStrokeWidth(2);
 setStroke(Color.WHITE);
 }
}

Bullet class:

class Bullet extends Polyline{
//Tag constants to identify if the bullet is from the alien or player
public final static String ALIEN = "ALIEN";
public final static String PLAYER = "PLAYER";
double increment;
String tag;
//Event handler that moves the bullet
EventHandler<ActionEvent> bulletMovement = e -> this.setTranslateY(this.getTranslateY() - increment);
Timeline bulletTimeline = new Timeline(new KeyFrame(Duration.millis(10), bulletMovement));
//Set bullet position, the movement increment, the tag, and style
Bullet(double positionX, double positionY, double increment, String tag){
 super(0.0, 0.0, 0.0, 10.0);
 setTranslateX(positionX);
 setTranslateY(positionY);
 this.increment = increment;
 this.tag = tag;
 setStroke(Color.WHITE);
 bulletTimeline.setCycleCount(Timeline.INDEFINITE);
 bulletTimeline.play();
 }
}

Answer 1

Thanks for sharing your code.

Just a few things coming to my mind when skimming over it:

Inappropriate inheritance

In OOP we create subclasses to extend the behavior of a parent class. Your classes Bullet and Alien do not extend the behavior of their parent classes but only configure them.

violation of information hiding / encapsulation

Your class GameManager has a property gamePane that is directly accessed from your class MyProj. Since you instantiate GameManager immediately before this access in MyProj you could instead instantiate the Panel object in MyProj and pass it as constructor parameter to GameManager.

Inline comments

Most of your inline comments are just useless because they repeat what the code itself already expresses (or could express with more careful naming). But some of them are lies:

 //Set bullet position, the movement increment, the tag, and style
 Bullet(double positionX, double positionY, double increment, String tag){

No! This is not a setter method, its the Constructor of that class that might (but shouldn't) set something as a side effect.

Work in constructors

Constructors should not have any logic. They should just assign their parameters to (final) member variables.

That's it.

Any calculation should be done outside the constructor and only results should be passed in.

---

Disclaimer

This is just my personal opinion and may be completely wrong.

• puppeteer: Thanks for the tips. I think you make a good point especially with the matter of inappropriate inheritance. I definitely should have thought about that more, along with the lack of encapsulation/need thereof for the gamePane object.