HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Classes for a small space invaders game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
spacegamesmallforclassesinvaders

Problem

Below are some of the classes I have written for a small space invaders game. It's not finished but it's at the bare bones stage. I am still learning how to properly use the Slick2D library. I know where there are some problems but that is due to my lack of full knowledge of how Slick2D works, so I had to think of a workaround for the time being.

I have only created this code so you can all check it out and tell me where I am going wrong and what I am doing right. I'd also like some feedback on the code in general, such as if it is clean, if it is organized, if the naming conventions for methods and variables helpful, and other things like that. I don't know if I am a good programmer or not so hopefully this may shine some light on the situation.

Entity class

```
package com.emeraldzonegames.jinvaders.entities;

import java.awt.Rectangle;

import org.newdawn.slick.GameContainer;
import org.newdawn.slick.Image;
import org.newdawn.slick.Graphics;

public abstract class Entity
{
public Image entityImage;
public int x, y, width, height,speed;
public Rectangle entityRect;

/**
* Method that loads an image for each entity
* @param imageName
*/
public void loadImage(String imageName)
{
try
{
entityImage = new Image("Assets/Graphics/"+imageName+".png");
}
catch(Exception e)
{
e.printStackTrace();
}
}

//Getters and setters
public void setPosition(int x, int y)
{
this.x = x;
this.y = y;
}

public void setDimenseions(int width, int height)
{
this.width = width;
this.height = height;
}

public void setSpeed(int speed)
{
this.speed = speed;
}

public int getX()
{
return x;
}

public int getY()
{
return y;
}

public int getWidth()
{
return width;
}

public int getHeight()
{
return height;
}

public int getSpeed()
{
return speed;
}

public Image getImage()
{
return entityImage;
}

/**
* Creates a rectangle for the entity
*/
public void createRectangle()
{
entityRect = new Rectangle(ge

Solution

Caveat: I am not a Java programmer. I am an ActionScript programmer. The two languages are somewhat related, and basic design principles should carry across.

At first glance, I don't see anything that would scare me away from hiring you if it were my decision. I would say that most of this code is better/cleaner than code I have seen from Computer Science graduates who have been programming 10 or more years.

So, what follows are just some observations that I have after 15 years or so of developing in AS + a few other languages.

  • Your Entity looks to me like an Abstract Class. Doesn't Java have formal support for these? You may or may not get brownie points for making this an Abstract Class.



  • setDimenseions--typo. Your IDE is clearly helping you here, but if you are using this for portfolio code, why make mistakes you don't need to make?



  • Your entityLogic method is getting information it doesn't need. Your Player Entity is the only one where you've fully fleshed out and used this method, and all you need there is the input, not the full GameContainer. If you need the full GameContainer, why not store it, rather than pushing it in every loop? I think at some level you realize how bad this is, since you're completely ignoring the method on Bullet. Yes, I know you built this off Slick2D's example code, but if Slick2D jumped off a cliff, would you?



  • Having renderEntity on Entity violates Single Responsibility Principle IMO. This Class reads more to me like a Value Object than a View Object, so it should not be performing View type activities. Again, you've followed less than stellar example code. Instead, just have your Main Class do something like g.drawImage(e.getImage(), e.getX(), e.getY()), or you could even have an EntityGraphics Class that can take an entity and draw it. Either of these solutions handily eliminates the unneccessary dependency both on Graphics and that horrible static debugMode throughout the Entity inheritance chain.



  • In Entity, sometimes you refer internally to the private storage variables, and sometimes you call the "getters." Why?



  • If you can't come up with a more descriptive JavaDoc comment for Player than what you used, you're probably better off leaving it off. We all know what Classes are for.



  • I'd go with a constructor argument for the image name, rather than a static constant in your Entity SubClasses. What if you want a Player that's a submarine? Consider this idea for the other values you're coming up with "out of thin air" in the constructor.



  • As I said, I see your Entity more as a VO, so Player shouldn't be responsible for creating its own bullets. What if you wanted to have more than one Player, with a global Object Pool of Bullets that recycles when they are taken off screen? I think you need to think a bit more about who should be responsible for what, where the Bullets are concerned. You have a mish-mash where some of it is handled by the Player and some of it is handled by the Main game, yet none of it goes through the "official" channel Entity should use for updating itself, entityLogic. Since your Main Class already concerns itself with looping through the Bullets, I'd probably relegate Player to simply storing the list of Bullets, and allow Main or some new Class to handle that.



  • Enemy--all that commented out code implies you're not a confident user of version control. Either fix it or take it out.



  • All those try/catch statements. Is there something specific that you expect could cause an error here? If so, test for that and fix it if possible (for example, null values).



Keep in mind, these are the type of suggestions I would offer to a very experienced developer, if I thought he/she was open to hearing it. I suspect that for most jobs, especially entry-level jobs, these points would be completely irrelevant. Note that if you take my suggestions that would have you deviating from the Slick2D example code, you might wind up shooting yourself in the foot with developers who feel it's better to stick to established norms for a variety of reasons, some of them good.

I think your code shows solid competence, and I wouldn't hesitate to hire you if all the other parts of the interview and hiring process lined up. Good luck!

Context

StackExchange Code Review Q#20551, answer score: 7

Revisions (0)

No revisions yet.