0

I'm creating a game where thats turn-based. In this game - I have a grid of N x N size.

I thought that it would be a great idea to use a 2D-boolean array to represent the binary states of all the cells in this grid.

Imagine:

// T = true | F = false
---------------------
| F |   |   |   |   |
---------------------
|   | T |   |   |   |
---------------------    // i.e. grid[0][0] = false
|   |   |   |   |   |
---------------------
|   |   |   |   |   |
---------------------

Each square can either be true or false. The rules of the game are not important... Just note that each cell can be true or false.

This is what I tried to implement:

public class Life {
    Boolean[][] grid;

    public Life(int x, int y, Boolean status) {
        if(!this.grid[x][y]) {
            this.grid = new Boolean[x][y];
        }
        this.grid[x][y] = status;
    }
}

Which I would instantiate like this:

new Life(0,0,false);
new Life(2,1,true);

However, when I do this, my program crashes and I'm not sure as to what I'm doing wrong. Any help is appreciated.

Caused by: java.lang.NullPointerException at Life.(Life.java:6)

Joel
  • 5,732
  • 4
  • 37
  • 65
  • In constructor You are trying to access the index in the array that was not initialized. That's why You are getting NPE. – Dominik Wosiński Apr 08 '19 at 18:43
  • @DominikWosiński I see, but how would i eg. set `grid[0][0] = true` ? – Joel Apr 08 '19 at 18:44
  • @Joel: use `boolean` primitive which is `false` by default. `Boolean` is an object and is `null` in your grid – Cratylus Apr 08 '19 at 18:46
  • @Cratylus I'm getting the same error unfortunately. – Joel Apr 08 '19 at 18:47
  • @Joel: You haven't initialized the array. Check Pavel Smirnov answer – Cratylus Apr 08 '19 at 18:52
  • 1
    @Joel: Also `grid` should be `static` – Cratylus Apr 08 '19 at 18:53
  • when you say `if(!this.grid[x][y])` you are trying to access the element [x][y], but the array has not been initialized yet. Instead it should simply be `if(this.grid != null)` – Chris Rollins Apr 08 '19 at 19:07
  • and Cratylus is probably correct. If you want each Life object to share the same grid, then it should be marked `static`. that appears to be your intent. – Chris Rollins Apr 08 '19 at 19:08
  • I think everyone is trying to reinvent the wheel here when Life should be a very simple object class with setStatus and getStatus methods to access elements in the array. We shouldn't be setting any statuses in the constructor. – brandonx Apr 08 '19 at 19:26

4 Answers4

1

You are mixing a constructor with a setter. It looks like you are trying to set the whole grid to whatever status is set to. That won't work for a couple of different reasons. First off:

this.grid[x][y] = status;

actually is trying to set the element at x, y coordinate to status. It will actually try to do this when you instantiate a new grid and will return an array out of bounds exception because of zero indexing. If you set a grid to 1, 1 size and then try to access element [1, 1], that does not exist. the only element in a [1, 1] grid is [0, 0].

You need to separate out the constructor and setter method like this:

public class Life {
    Boolean[][] grid;

    public Life(int xMax, int yMax) {       
            this.grid = new Boolean[xMax][yMax];
    }

    public void setStatus(int x, int y, Boolean status) {
            this.grid[x][y] = status;
    }

    public boolean getStatus(int x, int y) {
            return this.grid[x][y];
    }
}

edit: as others are mentioning, if you want your grid to default to false, you should use boolean rather than Boolean.

brandonx
  • 228
  • 2
  • 13
0

You're accessing grid field in the constructor, which is not instantiated yet.

What you probably want to do is:

public class Life {
    boolean[][] grid;

    public Life(int x, int y, boolean status) {
        grid = new boolean[5][5]; //where 5 is the size of the matrix: 5x5
        grid[x][y] = status;
    }
}

Note: I'm using the primitive boolean instead of Boolean object, since creating an object is redundant in your case.

UPDATE:
If Life is supposed to be one cell only, then it would be better to declare grid field as static and instantiate it only once. Any action with the cells could be peroformed via getters\setters:

public class Life {
    static boolean[][] grid = new boolean[5][5]; //where 5 is the size of the matrix: 5x5;

    public void setStatus(int x, int y, boolean status) {
        grid[x][y] = status;
    }

    public boolean getStatus(int x, int y) {
        return grid[x][y];
    }
}
Pavel Smirnov
  • 4,611
  • 3
  • 18
  • 28
  • I noticed that you removed `this.` also. Anyway. Interesting, but I dont really understand **why** i need to instantiate my grid with the gridsize... hmm. Thought I could just instantiate one cell at a time. – Joel Apr 08 '19 at 18:53
  • @Joel, well, before you can use any cell in a 2d array, you have to instantiate the whole array first, allocate memory for all the values. If you want to create only one cell at a time and access it by a composite key of `x` and `y`, you should probably look at a different data structure, like `HashMap`, for example, where `x` and `y` are keys and `Boolean` is a value. – Pavel Smirnov Apr 08 '19 at 18:56
  • @Joel you can set the size of the array using x and y parameters rather than the integer literals in this answer. – brandonx Apr 08 '19 at 18:58
  • Urgh... not sure I like the way Java is dealing with this. Memory-abuse.. Would be much nicer if I could allocate a spot anywhere in the array as I please. Anyway, thanks for the answer. Approved. – Joel Apr 08 '19 at 18:59
  • @brandonx, I presume, `x` and `y` are just coordinates to set the boolean value, not the size of the whole array. – Pavel Smirnov Apr 08 '19 at 19:00
  • @Joel, you're welcome :) You can also consider using List>. It's almost the same, but hmm.... is a resizable array of booleans. With this approach you can access values by `list.get(x).get(y);` But this is much more memory consumptive. – Pavel Smirnov Apr 08 '19 at 19:02
  • @PavelSmirnov your presumption is correct. Which is why I'm having a hard time digesting this. I'll look at different options eventually, but for now, i'll live with this solution. Thanks. – Joel Apr 08 '19 at 19:04
  • @PavelSmirnov I just don't feel like this is the right way to approach the problem. Life is the actual grid and therefore public Life should be the constructor for that grid. Your method will instantiate a new grid with every call and will assign one value of the grid at x, y to status. I don't see the use in that. A traditional object class with constructor and setter and getter for each element in the grid is much more useful and for someone new to java is an essential thing to understand. – brandonx Apr 08 '19 at 19:11
  • @Joel: `why i need to instantiate my grid with the gridsize...` you don't if you declare it `static` and initialize it once in the declaration – Cratylus Apr 08 '19 at 19:11
  • @brandonx, if `Life` is just one cell, then yes, grid should be instantiated only once. As a static field, or a separate object, whatever. It's quite not clear what `Life` is here, but I agree, setters and getters would be a better approach. – Pavel Smirnov Apr 08 '19 at 19:16
  • @PavelSmirnov, what I meant was that in this approach there is no way to further modify or evaluate the grid after the initial instantiation. Further, instantiating an array and setting one element of that array within the constructor doesn't seem to make a lot of sense. – brandonx Apr 08 '19 at 19:20
  • @Joel I would also add that the amount of memory needed for a boolean 2d array is trivial until you get to a massive size. I wouldn't worry too much about that. – brandonx Apr 08 '19 at 19:22
  • Not sure what I'm doing wrong but I pretty much always get: `Exception in thread "JavaFX Application Thread" java.lang.ArrayIndexOutOfBoundsException: -2` with this approach. I've increased the gridsize and I'm calling it using the 2nd example with getStatus – Joel Apr 08 '19 at 19:50
  • @Joel, you trying to get a cell at index -2, which obviously does not exist. You should inspect your code and make sure you pass the right parameters into the method. – Pavel Smirnov Apr 08 '19 at 20:04
  • @PavelSmirnov Ok, how can i check a neighbouring cells values then? I'm doing getStatus(x+1)(y+1) :( Obviously it doesnt exist. But how can I prevent this checking from crashing? – Joel Apr 08 '19 at 20:18
  • @Joel, since you know the size of the array, you should stop incrementing x and y once you hit a border. – Pavel Smirnov Apr 08 '19 at 20:23
0

The problem is caused by the fact that Boolean defaults to null. Thus, when you try to do:

if(!this.grid[x][y]) {

you're really doing

if(!null) {

which is invalid Java, and results in a NullPointerException. If you want the elements of your grid to default to false, then use boolean[][] instead of Boolean[][], since boolean primitives cannot be null, and default to false:

public class Life {
    static boolean[][] grid;

    public Life(int x, int y, boolean status) {
        this.grid[x][y] = status;
    }
}

If your constructor needs to take a Boolean object, then you can use a ternary to check for null:

public Life(int x, int y, Boolean status) {
        this.grid[x][y] = status == null ? false : status;
    }
Jordan
  • 2,273
  • 9
  • 16
  • Actually, i wanted to check if it had been instantiated. Bad habit of doing `!var` from c# i guess. Also, your last two examples crash. – Joel Apr 08 '19 at 18:55
-1

You are calling this line:

if(!this.grid[x][y]) {

In an uninitialized matrix, declared in the first line:

Boolean[][] grid;

You are also overriding the value of the grid for each new value you add, which I'm pretty sure it's not your intention.

You should be initializing the matrix in the constructor first, like this:

public Life(int x, int y) {
  grid = new boolean[x][y];
}

Then only referencing the values stored like this:

public addValue(int x, int y, Boolean status) {
    grid[x][y] = status;
}
Matias Fuentes
  • 439
  • 3
  • 8