0

I am working on an extremely basic game. However when I try to create the array i am running into errors. The error is index out of bounds. However I thought I fixed it by adding the -1 to make sure I don't go out of bounds. can someone tell me, or give me a clue as to what I did wrong?

package gameProject;

public class world {
int numEnemies, numBosses;
int [][] world = new int[10][10];

public world(){
    int[][] world  = createArray(10,10);
    populateWorld(world);
}

private int[][] createArray(int inX, int inY){
    //create the array that holds world values
    int[][] world = new int[inX][inY];
    //initialize the world array
    for(int i = 0; i < world.length - 1; i ++){
        for(int j = 0; j < world[0].length - 1; j++){
            world[i][j] = 0;
        }
    }

    return world;
}

private void populateWorld(int[][] world){
    for(int i = 0; i < world.length - 1; i++){
        for(int j = 0; j < world[0].length - 1; i++){
            world[i][j] = 0;
        }
    }

}
}
2
  • At what line does the error message say is the problem? Commented Jan 19, 2016 at 21:08
  • 1
    Doing i < world.length - 1 instead of i < world.length skips the last element, because < is not an inclusive range. Your "fix" does not seem correct. Also, if you are only initializing to 0, you do not need to do anything. Java auto-initializes arrays to 0 on creation. Commented Jan 19, 2016 at 21:11

4 Answers 4

3

In your populateWorld method, change

for(int j = 0; j < world[0].length - 1; i++)

to

for(int j = 0; j < world[0].length - 1; j++)

You keep incrementing the wrong counter, going eventually out of its bounds. (10)

(PS: you don't need the length - 1 in your loops' condition, just length would do)

Sign up to request clarification or add additional context in comments.

Comments

1

The error is in

for (int j = 0; j < world[0].length - 1; i++)

you should write

for (int j = 0; j < world[0].length - 1; j++)

instead.

Note that you can reduce your code a little bit:

You create the array for member World.world twice. Also the elements of an int array are already initialized to 0 so you don't need to do this explicitly.

2 Comments

Ahh thank you something simple, but completely obvious. Thanks you very much!!!!
@Kevin also added some suggestions for improvement
0

You should just do

private int[][] createArray(int inX, int inY) {
    int[][] world = new int[inX][inY];
    for (int i = 0; i < inX; i++)
        for (int j = 0; j < inY; j++)
            world[i][j] = 0;
    return world;
}

You never actually need to check the length of the world array, because the length was already passed in as a parameter value.

And then also

private void populateWorld(int[][] world) {
    for (int i = 0; i < world.length; i++)// v     error 'i' should be 'j'
        for (int j = 0; j < world[i].length; j++) // <- error on this line
            world[i][j] = 0;
}

Comments

0

Your basic problem is that you're incrementing the wrong loop variable. Why? Because you're far off from any clean code. Lemme show you how clean coding is done:

  • class names start with a capital letter, method and variable name with lower case letters
  • you might prefix your variables with their scope ('m' for member, 'p' for parameter, nothing for local variables). Saves you the all-time-reference to 'this'. Strongly depends on your code style. I highly suggest doing it, keeps your code clean and really easy to debug.
  • use the final and private keywords where possible
  • use descriptive variable names. Here especially x and y for loop variables, as you're abstracting a 2d-plane

Some more considerations:

  • usually games grow more complex. Usually simple primitives (like your int-array) will not suffice for long to store all relevant information. Use classes like Cell
  • use enums so you can lose magic numbers => coding, reading and debugging made a lot easier

So - after a lot of talk - here's the final code:

package gameproject;

/**
 * Use comments like this to describe what the classes purpose is.
 * Class comment is the most important one. If you can't tell what a method/variable is doing by its name, you should also comment methods and/or variables!
 * @author JayC667
 */
public class World {

    /*
     * STATIC part of the class - keep separated from object code
     */

    // you could/should also put these static classes to their separate files and make em non-static
    /**
     * Denotes, what a {@linkplain Cell} is occupied with
     */
    static public enum CellType {
        EMPTY, //
        BOSS, //
        ENEMY
    }

    /**
     * Represents a single cell within the world. Stores information about its coodrinates (redundant) and its occupator (see {@linkplain CellType})
     * @author JayC667
     */
    static private class Cell { // use cell to store data for you
        public final int    mX;                         // x and y are actually not useful at the moment, you could also remove them
        public final int    mY;
        private CellType    mCellType   = CellType.EMPTY;

        public Cell(final int pX, final int pY) {
            mX = pX;
            mY = pY;
        }

        public CellType getCellType() {
            return mCellType;
        }
        public void setCellType(final CellType pCellType) {
            mCellType = pCellType;
        }
    }

    // when possible, make methods static, unless you unnecessarily blow up the parameter list
    // this is a typical demo for a factory method
    static private Cell[][] createWorld(final int pWidth, final int pHeight) {
        final Cell[][] newWorld = new Cell[pWidth][pHeight];
        for (int y = 0; y < pHeight - 1; y++) {
            for (int x = 0; x < pWidth - 1; x++) {
                newWorld[y][x] = new Cell(x, y);
            }
        }
        return newWorld;
    }

    /*
     * OBJECT part of the class - keep separated from static code
     */

    private final Cell[][]  mWorld;
    private final int       mWorldWidth;
    private final int       mWorldHeight;
    private final int       mNumberOfEnemies;
    private final int       mNumberOfBosses;

    public World(final int pWidth, final int pHeight, final int pNumberOfEnemies, final int pNumberOfBosses) {
        if (pWidth < 1 || pHeight < 1) throw new IllegalArgumentException("World width and height must be greater than 0!");
        if (pNumberOfEnemies < 0 || pNumberOfBosses < 0) throw new IllegalArgumentException("Enemy and boss counts must not be negative!");
        if (pWidth * pHeight < pNumberOfEnemies + pNumberOfBosses) throw new IllegalArgumentException("World is too small for all the bad guys!");

        mWorldWidth = pWidth;
        mWorldHeight = pHeight;
        mNumberOfEnemies = pNumberOfEnemies;
        mNumberOfBosses = pNumberOfBosses;
        mWorld = createWorld(pWidth, pHeight);
        populateWorld();
    }

    // refers to many member variables, so not static (would only blow up parameter list)
    private void populateWorld() {
        for (int i = 0; i < mNumberOfBosses; i++) {
            final Cell c = getRandomCell(CellType.EMPTY);
            mWorld[c.mY][c.mX].setCellType(CellType.BOSS);
        }
        for (int i = 0; i < mNumberOfEnemies; i++) {
            final Cell c = getRandomCell(CellType.EMPTY);
            mWorld[c.mY][c.mX].setCellType(CellType.ENEMY);
        }
    }

    private Cell getRandomCell(final CellType pCellType) {
        while (true) { // TODO not a good, but simple solution; might run infinite loops
            final int randomX = (int) (mWorldWidth * Math.random());
            final int randomY = (int) (mWorldHeight * Math.random());
            if (mWorld[randomY][randomX].getCellType() == pCellType) return new Cell(randomX, randomY);
        }
    }

}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.