7
\$\begingroup\$

Some days ago, I posted a question here: Field class as basis for a role playing game

I payed respect to tips I find useful and made a further development of my Field-classes. These classes have to serve as a basis for a role playing game later.

Now I tested my classes with a little snake game. You can download the program here.

The Field-"API" consists of three classes:

Entity: These class is a basis for all elements that have to appear on a field. You can use this class directly or as a basis for you own Entity-classes.

public class Entity {
    protected String name;
    protected char symbol;
    protected int xCoordinate;
    protected int yCoordinate;
    // decides if entity can move around on a field
    protected boolean moveable;
    // decides if other entities can walk on that entity on a field
    protected boolean walkable;
    // decides if entity can appear on several positions simultaneously
    protected boolean multifarious;

    public Entity(String name, char symbol, boolean moveable, boolean walkable, boolean multifarious) {
        this.name = name;
        this.symbol = symbol;
        this.moveable = moveable;
        this.walkable = walkable;
        this.multifarious = multifarious;
    }

    public Entity(String name, char symbol, boolean moveable, boolean walkable) {
        this(name, symbol, moveable, walkable, false);
    }

    public boolean isMoveable() {
        return moveable;
    }

    public boolean isWalkable() {
        return walkable;
    }

    public boolean isMultifarious() {
        return multifarious;
    }

    public String getName() {
        return name;
    }

    public char getSymbol() {
        return symbol;
    }

    public int getXCoordinate() {
        return xCoordinate;
    }

    // if you want to change both x and y, you should prefer updateCoordinates(x, y) for better readability
    public void setXCoordinate(int x) {
        this.xCoordinate = x;
    }

    public int getYCoordinate() {
        return yCoordinate;
    }

    // if you want to change both x and y, you should prefer updateCoordinates(x, y) for better readability
    public void setYCoordinate(int y) {
        this.yCoordinate = y;
    } 

    public void updateCoordinates(int x, int y) {
        setXCoordinate(x);
        setYCoordinate(y);
    }
}

Field: These class contains the entities and manages and displays them.

import java.util.List;
import java.util.ArrayList;

public class Field {
    private int height;
    private int width;
    // only the last entity of a list is drawed on the field
    // field is built like that: [height][width] -> List of Entities
    private List<List<List<Entity>>> positions;
    private boolean multipleEntitiesOnPosition;
    private char emptyPositionRepresentation;
    private List<Entity> placedEntities;

    public Field(int height, int width, boolean multipleEntitiesOnPosition, char emptyPositionRepresentation) {
        this.height = height;
        this.width = width;

        positions = new ArrayList<List<List<Entity>>>();
        for (int i = 0; i < height; i++) {
            positions.add(new ArrayList<List<Entity>>());
            for (int j = 0; j < width; j++) {
                positions.get(i).add(new ArrayList<Entity>());
            }
        }

        this.multipleEntitiesOnPosition = multipleEntitiesOnPosition;
        placedEntities = new ArrayList<Entity>();
        this.emptyPositionRepresentation = emptyPositionRepresentation;
    }

    public Field(int height, int width, boolean multipleEntitiesOnPosition) {
        this(height, width, multipleEntitiesOnPosition, '.');
    }

    public int getHeight() {
        return height;
    }

    public int getWidth() {
        return width;
    }

    public List<Entity> getEntities() {
        // user is not allowed to modify list, thats why a copy is given
        return new ArrayList<Entity>(placedEntities);
    }

    private boolean checkForValidCoordinates(int x, int y) {
        if (x >= height || y >= width || x < 0 || y < 0) {
            return false;
        }
        return true;
    }

    public boolean addEntity(Entity entity, int x, int y) {
        if (!checkForValidCoordinates(x, y)) {
            return false;
        }

        // check if entity is already on field
        if (placedEntities.contains(entity) && !entity.isMultifarious()) {
            return false;
        }

        List<Entity> entityList = positions.get(x).get(y);

        // check if entity is already on that position
        if (entityList.contains(entity)) {
            return false;
        }

        // check if rule about multiple entities on position is violated
        if (!multipleEntitiesOnPosition && !entityList.isEmpty()) {
            return false;
        }

        // check if another entity is already on field that can not be passed
        for (Entity ent : entityList) {
            if (!ent.isWalkable()) {
                return false;
            }
        }

        // check if entity gets placed on another entity that is not moveable
        for (Entity ent : entityList) {
            if (!ent.isMoveable()) {
                return false;
            }
        }

        placedEntities.add(entity);
        entityList.add(entity);

        entity.updateCoordinates(x, y);

        return true;
    }

    public boolean removeEntity(Entity entity) {
        if (!placedEntities.contains(entity)) {
            return false;
        }

        placedEntities.remove(entity);

        for (int i = 0; i < height; i++) {
            for (int j = 0; j < width; j++) {
                List<Entity> entities = positions.get(i).get(j);
                if (entities.contains(entity)) {
                    entities.remove(entity);
                    if (!entity.isMultifarious()) {
                        break;
                    } 
                }
            }
        }
        return true;
    }

    public boolean moveEntity(Entity entity, int newX, int newY) {
        if (!checkForValidCoordinates(newX, newY)) {
            return false;
        }

        // check if another entity is already on field that can not be passed
        for (Entity ent : positions.get(newX).get(newY)) {
            if (!ent.isWalkable()) {
                return false;
            }
        }

        // check if entity is on field
        if (!placedEntities.contains(entity)) {
            return false;
        }

        // check if entity is moveable
        if (!entity.isMoveable()) {
            return false;
        }

        positions.get(entity.getXCoordinate()).get(entity.getYCoordinate()).remove(entity);
        positions.get(newX).get(newY).add(entity);
        entity.updateCoordinates(newX, newY);
        return true;
    }

    public boolean moveEntity(Entity entity, Direction direction) {
        switch (direction) {
            case UP:
                return moveEntity(entity, entity.getXCoordinate() - 1, entity.getYCoordinate());
            case DOWN:
                return moveEntity(entity, entity.getXCoordinate() + 1, entity.getYCoordinate());
            case LEFT:
                return moveEntity(entity, entity.getXCoordinate(), entity.getYCoordinate() - 1);
            case RIGHT:
                return moveEntity(entity, entity.getXCoordinate(), entity.getYCoordinate() + 1);
        }
        return false;
    }

    public boolean hasPositionEntities(int x, int y) {
        if (positions.get(x).get(y).isEmpty()) {
            return false;
        }
        return true;
    }

    public List<Entity> getEntitiesOfPosition(int x, int y) {
        // user is not allowed to modify list, thats why a copy is given
        return new ArrayList<Entity>(positions.get(x).get(y));
    }

    public String toString() {
        StringBuilder returnValue = new StringBuilder();
        for (int i = 0; i < height; i++) {
            for (int j = 0; j < width; j++) {
                List<Entity> entities = positions.get(i).get(j);
                if (!entities.isEmpty()) {
                    char lastSymbol = entities.get(entities.size() - 1).getSymbol();
                    returnValue.append(lastSymbol);
                } else {
                    returnValue.append(emptyPositionRepresentation);
                }
            }
            returnValue.append('\n');
        }
        return returnValue.toString();
    }
}

Direction: This is just an enum that simplifies moving some entities.

public enum Direction {
    UP, DOWN, LEFT, RIGHT
}

To test the functionality of my classes and to test the ability of them to serve usefully for applications I made a "Snake-Game" where a player has to eat all snacks and escape from the snake. If you want, you can also give me hints how to improve these classes, I would be thankful!

Main.java

public class Main {
    public static void main(String[] args) {
        new Game().play();
    }
}

Game.java

import java.util.Scanner;

public class Game {
    public static Scanner scanner = new Scanner(System.in);

    public void play() {
        boolean run = true;
        while (run) {
            System.out.println("[1] Start new Game");
            System.out.println("[2] Exit Game");

            System.out.print("Input: ");
            String input = scanner.next();
            System.out.println();

            switch (input) {
                case "1":
                    new SnakeGame().play();
                    break;
                case "2":
                    run = false;
                    break;
            }
        }
    }
}

SnakeGame.java

import java.util.Scanner;
import java.util.Random;
import java.util.List;

public class SnakeGame {
    private static Scanner scanner = new Scanner(System.in);

    private Field field;
    private Player player;
    private Snake snake;
    private int score;
    private int numOfSnacks;
    private int snacksEaten;

    public SnakeGame() {
        field = new Field(10, 20, false);
        player = new Player(field);
        snake = new Snake(field, player);

        field.addEntity(player, 1, 1);
        field.addEntity(snake, 9, 19);

        generateSnacks();

        score = 0;
        snacksEaten = 0;
    }

    private void generateSnacks() {
        numOfSnacks = 20;
        int numOfPlacedSnacks = 0;
        Random random = new Random();
        while (numOfPlacedSnacks < numOfSnacks) {
            Entity snack = new Entity("Snack", '#', false, true);
            int x = random.nextInt(field.getHeight());
            int y = random.nextInt(field.getWidth());
            if (!field.hasPositionEntities(x, y)) {
                field.addEntity(snack, x, y);
                numOfPlacedSnacks++;
            }
        }
    }

    public void play() {
        while (!snakeCatchedPlayer() && numOfSnacks != snacksEaten) {
            System.out.println("Score: " + score + "\n");
            System.out.println(field);
            player.move();
            checkIfPlayerEatSnack();
            boolean snakeMoved = snake.move();
            clearScreen();
            if (!snakeMoved) {
                System.out.println("The snake stumbled!\n");
            }
        }
        System.out.println(field);
        if (snakeCatchedPlayer()) {
            System.out.println("It catched you!\n");
        }
        if (numOfSnacks == snacksEaten) {
            System.out.println("Congratulations! You ate all snacks!");
        }
        System.out.println("Final score: " + score + "\n");
    }

    // checks if player and snake are on same position
    private boolean snakeCatchedPlayer() {
        boolean sameXCoordinate = player.getXCoordinate() == snake.getXCoordinate();
        boolean sameYCoordinate = player.getYCoordinate() == snake.getYCoordinate();

        if (sameXCoordinate && sameYCoordinate) {
            return true;
        }
        return false;
    }

    private void checkIfPlayerEatSnack() {
        List<Entity> entityList = field.getEntitiesOfPosition(player.getXCoordinate(), player.getYCoordinate());
        for (Entity ent : entityList) {
            if (ent.getName().equals("Snack")) {
                field.removeEntity(ent);
                score++;
                snacksEaten++;
            }
        }
    }

    private void clearScreen() {
        for (int i = 0; i < 20; i++) {
            System.out.println();
        }
    }
}

Player.java

import java.util.Scanner;

public class Player extends Entity {
    private static Scanner scanner = new Scanner(System.in);

    private Field field;

    public Player(Field field) {
        super("Player", 'P', true, true);
        this.field = field;
    }

    public void move() {
        System.out.print("Input: (w, a, s, d): ");
        String input = scanner.next();
        System.out.println();
        switch (input) {
            case "w":
                field.moveEntity(this, Direction.UP);
                break;
            case "a":
                field.moveEntity(this, Direction.LEFT);
                break;
            case "s":
                field.moveEntity(this, Direction.DOWN);
                break;
            case "d":
                field.moveEntity(this, Direction.RIGHT);
                break;
        }
    }
}

Snake.java

import java.util.Random;

public class Snake extends Entity {
    private static Random random = new Random();

    private Field field;
    private Player player;

    public Snake(Field field, Player player) {
        super("Snake", 'S', true, true);
        this.field = field;
        this.player = player;
    }

    // returns if snake was able to move
    public boolean move() {
        // able to move?
        if (random.nextInt(4) == 3) {
            return false;
        }

        if (player.getXCoordinate() > xCoordinate) {
            field.moveEntity(this, xCoordinate + 1, yCoordinate);
            return true;
        } else if (player.getXCoordinate() < xCoordinate) {
            field.moveEntity(this, xCoordinate - 1, yCoordinate);
            return true;
        }

        if (player.getYCoordinate() > yCoordinate) {
            field.moveEntity(this, xCoordinate, yCoordinate + 1);
            return true;
        } else if (player.getYCoordinate() < yCoordinate) {
            field.moveEntity(this, xCoordinate, yCoordinate - 1);
            return true;
        }

        return false;
    }
}
\$\endgroup\$
3
+50
\$\begingroup\$

I only got to the core classes, not the game implementation. Hopefully this will give you something to think about.

General

Classes that are not designed for extension should be explicitly marked as final. Variables that will not be reassigned should also be marked as final. This makes it easier for readers of your code.

I'm assuming this API is designed only for a console UI. You've made several decisions which tie you strongly to that format, as noted below.

Entity

Avoid extension where possible. Classes are limited to exactly one parent class, and you can design yourself into a corner. I'd suggest making Entity an interface and copying the implementation into an abstract class. Clients can then choose whether or not to use the base implementation.

Classes that should not be instantiable should be declared as abstract. Your design is unclear - some instances (Snake, Player) extend Entity, while others just are entities (Snack). The difference appears to be whether or not the Entity can move, but movement is not part of the contract of the Entity class.

None of the protected properties should be protected. You lose encapsulation of them - they no longer belong to Entity. All the variables are already available via accessors.

walkable is not a great property name. Something like blocksMovement would be more clear.

Encoding the symbol directly onto an Entity ties you pretty strongly to a console UI. symbol becomes meaningless if you add GUI support and clients don't implement a console version of their game.

The comments should all be Javadoc. Since This class is intended as part of your API, it should in general be much more strongly documented.

"multifarious" does not mean "can appear in multiple locations at the same time". The property is never used. Don't add support for features unless you're sure you need them, because you have to support them forever or break your clients. The entire concept of the property is dubious - how can an entity instance have a single x coordinate, a single y coordinate, and yet exist in multiple locations?

Field

As of recent Java versions, you no longer need to specify the generic type information on the RHS in most cases.

The constructor is easier to read if all the simple assignments occur before you build the positions variable.

Where possible, assign variables where they're defined.

Again you're directly tying yourself to a console with emptyPositionRepresentation. If it's your intent to do so, it should be mentioned. If not, it should be fixed.

In the constructor, it's a little cleaner to keep the reference to the created List around rather than calling get() j times.

You can't track the position of an Entity in both Entity and Field. It will confuse clients and they will get out synch. Let the Field handle this.

Good use of a defensive copy in getEntities(). Using Collections.unmodifiableList() is another option. Note that the entities themselves can still be modified by callers.

checkValidCoordinates can have its logic simplified to return !(x >= height ... or return (x >= 0) && (x < height) ... hasPositionEntities can likewise be simplified.

You're always checking !checkValidCoordinates. It would be easier if the method was areInvalidCoordinates.

Rather than dealing with a List<List<List<Entity>>>, it might be easier to deal with a Position[][], where Position is a wrapper for a List. You can add some simple methods to that new class which would clean up your code quite a bit.

addEntity is checking if the entity has been placed, and then it's checking the position to see if it's already there. The second check shouldn't be needed - if it's on the position, it should be tracked in the placed entities already.

Rather than tracking multipleEntities as a boolean, perhaps track a capacity for the position? Then you have a lot more flexibility than just one or many.

It would clean up your code if placedEntities was a Map of entities to their positions.

The validation checks are not consistent for adding vs. moving into a space. Specifically, add() checks to see if any of the entities in the position are moveable, for reasons that are not clear. If you decided to use a Position object, you could move that logic into one method on the Position.

getEntities() appears to be unused and can be removed.

You might consider throwing exceptions in some cases rather than just returning false. For instance, if the client tries to move something that's not on the field, an exception is harder to miss than a return value.

moveEntity isn't making sure that the entity is on the field.

Don't have a switch statement that does something for each possible enum value. Move that logic to the enum.

If you were to make all these changes, your code might look something like:

public abstract class AbstractEntity implements Entity {

    private final String name;
    private final char symbol;

    /** controls if this entity can move around on a {@link Field} */
    private final boolean moveable;

    /** controls if other entities can walk on that entity on a {@link Field} */
    private final boolean blocksMovement;

    public AbstractEntity(final String name, final char symbol, final boolean moveable, final boolean blocksMovement) {
        this.name = name;
        this.symbol = symbol;
        this.moveable = moveable;
        this.blocksMovement = blocksMovement;
    }

    public boolean isMoveable() {
        return this.moveable;
    }

    public boolean blocksMovement() {
        return this.blocksMovement;
    }

    public String getName() {
        return this.name;
    }

    public char getSymbol() {
        return this.symbol;
    }

}

public enum Direction {

    UP(-1, 0),
    DOWN(+1, 0),
    LEFT(0, -1),
    RIGHT(0, 1);

    private final int adjustX;
    private final int adjustY;

    private Direction(final int adjustX, final int adjustY) {
        this.adjustX = adjustX;
        this.adjustY = adjustY;
    }

    public int newX(final Position position) {
        return position.getXCoordinate() + this.adjustX;
    }

    public int newY(final Position position) {
        return position.getYCoordinate() + this.adjustY;
    }
}

public interface Entity {

    boolean isMoveable();
    boolean blocksMovement();
    String getName();
    char getSymbol();

}

public final class Field {

    private final int height;
    private final int width;

    // only the last entity of a list is drawn on the field
    // field is built like that: [height][width] -> List of Entities
    private final Position[][] positions;
    private final Map<Entity, Position> placedEntities = new HashMap<>();

    public Field(final int height, final int width, final int positionCapacity) {
        this(height, width, positionCapacity, '.');
    }

    private Field(
            final int height,
            final int width,
            final int positionCapacity,
            final char emptyPositionRepresentation) {

        this.height = height;
        this.width = width;

        positions = new Position[height][width];
        for (int i = 0; i < height; i++) {
            for (int j = 0; j < width; j++) {
                positions[i][j] = new Position(i, j, positionCapacity, emptyPositionRepresentation);
            }
        }
    }

    public int getHeight() {
        return this.height;
    }

    public int getWidth() {
        return this.width;
    }

    public boolean addEntity(final Entity entity, final int x, final int y) {
        if (this.areInvalidCoordinates(x, y)) {
            return false;
        }

        if (this.placedEntities.containsKey(entity)) {
            return false;
        }

        final Position position = positions[x][y];
        if (!position.canAddEntity()) {
            return false;
        }

        this.placedEntities.put(entity, position);
        position.add(entity);

        return true;
    }

    public boolean removeEntity(final Entity entity) {
        if (!placedEntities.containsKey(entity)) {
            return false;
        }

        final Position position = placedEntities.remove(entity);
        position.remove(entity);
        return true;
    }

    public boolean moveEntity(final Entity entity, final int newX, final int newY) {
        if (this.areInvalidCoordinates(newX, newY)) {
            return false;
        }

        if (!placedEntities.containsKey(entity)) {
            return false;
        }

        if (!entity.isMoveable()) {
            return false;
        }

        final Position newPosition = positions[newX][newY];
        if (!newPosition.canAddEntity()) {
            return false;
        }

        final Position oldPosition = this.placedEntities.put(entity, newPosition);
        oldPosition.remove(entity);
        newPosition.add(entity);

        return true;
    }

    public boolean moveEntity(final Entity entity, final Direction direction) {
        final Position position = this.placedEntities.get(entity);
        if (position == null) {
            return false;
        }

        return moveEntity(entity, direction.newX(position), direction.newY(position));
    }

    public boolean hasEntitiesAt(final int x, final int y) {
        return !this.positions[x][y].isEmpty();
    }

    public Collection<Entity> getEntitiesAt(final int x, final int y) {
        return this.positions[x][y].getEntities();
    }

    public Position getPositionForEntity(final Entity entity) {
        return this.placedEntities.get(entity);
    }

    public String toString() {
        final StringBuilder returnValue = new StringBuilder();
        for (int i = 0; i < this.height; i++) {
            for (int j = 0; j < this.width; j++) {
                returnValue.append(positions[i][j].asChar());
            }
            returnValue.append('\n');
        }
        return returnValue.toString();
    }

    private boolean areInvalidCoordinates(final int x, final int y) {
        return (x < 0) || (x >= this.height) || (y < 0) && (y >= this.width);
    }
}

public final class Position {

    private final int xCoordinate;
    private final int yCoordinate;
    private final int capacity;
    private final char emptyPositionRepresentation;
    private final List<Entity> entities = new ArrayList<>();

    public Position(
            final int xCoordinate,
            final int yCoordinate,
            final int capacity,
            final char emptyPositionRepresentation) {
        this.xCoordinate = xCoordinate;
        this.yCoordinate = yCoordinate;
        this.capacity = capacity;
        this.emptyPositionRepresentation = emptyPositionRepresentation;
    }

    public int getXCoordinate() {
        return this.xCoordinate;
    }

    public int getYCoordinate() {
        return this.yCoordinate;
    }

    public boolean contains(final Entity entity) {
        return this.entities.contains(entity);
    }

    public boolean isEmpty() {
        return this.entities.isEmpty();
    }

    public boolean add(final Entity entity) {
        if (this.capacity > this.entities.size()) {
            return false;
        }
        return this.entities.add(entity);
    }

    public boolean remove(final Entity entity) {
        return this.entities.remove(entity);
    }

    public boolean canAddEntity() {
        if (this.capacity > this.entities.size()) {
            return false;
        }

        for (final Entity entity : this.entities) {
            if (entity.blocksMovement()
                    || !entity.isMoveable()) {
                return false;
            }
        }

        return true;
    }

    public Collection<Entity> getEntities() {
        return Collections.unmodifiableCollection(this.entities);
    }

    /**
     * @return a character representation of the entities in this position.
     */
    public char asChar() {
        if (entities.isEmpty()) {
            return this.emptyPositionRepresentation;
        }

        return this.entities.get(entities.size() - 1).getSymbol();
    }


}
\$\endgroup\$
1
\$\begingroup\$

The way to move things is extremely restricted. Instead of marking entities moveable (mobile?), encapsulate the behaviour of mobility behind an interface and inject a suitable implementation to each entity. When the time comes to move an entity, whoever is responsible for triggering it, fetch the mobility implementation from the entity and have it perform the motion on the entity in question.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.