• Post Reply Bookmark Topic Watch Topic
  • New Topic

Creating Conway's Game of Life - Index out of Bounds problem?  RSS feed

 
Aarden Axford
Greenhorn
Posts: 28
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The question pretty much says it all. My problem seems to be when adding the neighbours, I am always getting a Index out of Bounds problem. I know this is because the code is reaching for the edge of the table, for example if the column, i = 0 and the statement says to perform i - 1 and return it, then we are going to have a problem. Similarly with anything like j = 20, j + 1 (as the grid only has 20 spaces)
I understand the problem, but I am unsure of how to solve it. I have tried messing around with the if statements, but I continue to get the 'out of bounds' problem... This is homework, before you ask, so in that regard I want to avoid a cut/paste answer if I can help it. Thanks for any help/hints !
 
fred rosenberger
lowercase baba
Bartender
Posts: 12563
49
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Where - EXACTLY - are you getting the OOB error? the stack trace tells you the exact line.

Once you know the line, put in some System.out.println statements to see what all your variable are.

Confirm the array really IS the size you think it is.

Note: there are a lot of magic numbers in your code. You would be better served defining a constant like "boardSize" and using that everywhere, instead of the "20" you have liberally sprinkled through your code.
 
Knute Snortum
Sheriff
Posts: 4276
127
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
There are many places in the code where you have something <= 20, where something is an index into the array. There are two problems with this: one is that you're going to get an ArrayIndexOutOfBoundsException when something == 20. The other is that 20 is what we call a magic number, that is, what does it mean? In general, if you have a number in your code that isn't 0 or 1, you should use a constant or variable to document what the number is. Is your case, the number is the size of the array, so you could do something like:



Then pick either something < ARRAY_SIZE or something < playingBoard.length and change your code accordingly. Also, if you are going to look at the next index, the test is something < ARRAY_SIZE - 1.

Now, for the readability of your code, do two things. One, lines should not be much longer that 80 characters. And comments at the end of lines should be short. Put longer comments above the line they're commenting on.
 
Stefan Evans
Bartender
Posts: 1837
10
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
#1: Check your loops.
In some cases you have used: for (int i = 0; i < 20; i++)
In others you have used: for (int i = 0; i <= 20; i++)
That is probably the root cause of the problem.

An observation: A whole lot of the conditions in your if statements are redundant.
If you are in a loop specifying the value of i will be from 1 to 20, then checking if i is between 1 and 20 is not necessary.
Having redundant conditions in there makes the code harder to read.

You want to check the border conditions:
In the top row - don't look for a neighbour above
In the leftmost column - don't look for a neighbour to the left
In the bottom row - don't look for a neighbour underneath
In the rightmost column - don't look for a neighbour to the right.


Here is a suggestion to completely avoid index out of bounds exceptions:
Put a 'border' around your grid, such that there is always a row or column above or below any valid square. So if you have a 3x3 board, construct a 5x5 array. Your actual board is indexes 1-3. Anything in row 0 or 4 should always be "dead". If you iterate from 1 to 3(inclusive) then you can always do a +1 or -1 without worrying about falling off the 'edge'.
It requires some more setup, and for you to be careful never to actually make the "border" cells live, but it can remove complexity in other areas.

 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It doesn't look like you're calculating the next generation correctly either since you're using only the one array, playingBoard, to store both the current generation population and the next generation population. By doing so, cells that you visit later in the loop will already have some of the next generation's state around them, which is incorrect. You need something like a nextGeneration method that returns a new array based on the current generation, which you pass as a parameter.
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
By using a String[][] to represent the board you are making the display of the population take precedence in your design and you miss out on using a more convenient logical representation of the Game of Life population. IMO, the code would be cleaner and easier to work with if you use a boolean[][] where true indicates that there is a living organism occupying a cell and false when there is none. When you display the board, you just display an appropriate character. That is, if the cell contains true, then display an "O", otherwise display a "."

If you had a boolean[][], you could use the expression board[row][col] ? "O" : "." for display purposes.

And if you do as Stefan suggested and put a "border" around your board so that you don't have to deal with Index Out Of Bounds exceptions, then you just use a number of similar expressions to count how many neighbors each cell has.
 
Aarden Axford
Greenhorn
Posts: 28
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you very much for your help. Here is my new (and hopefully improved? *cringes*) code... I am now realising that my "neighbours" function may be flawed, as the nextGen board isn't printing properly. I am currently messing around with to try and understand how to best fix it.
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Not trying to make you feel too bad but this code is what's sometimes referred to as a "Big Ball of Mud" -- everything is glommed together in one method. Imagine a book without chapters or paragraphs, just continuous text, sentence after sentence without any logical breaks or groupings. A book like that would not be very fun to read, right? Same thing goes with code that's just one long series of statements with no apparent logical breaks or groupings.

See this example of a Composed Method - the bottom version is much shorter and tells a clear "story" of what's happening. The code in the upper portion is chock-full of details but it's hard to tell what it's really doing at a glance. The example doesn't show you where the rest of the code went after it was refactored but the details of the conditional expression and the for-loop have, in fact, been hidden inside the methods that are standing in their place. That is, the atCapacity() and grow() methods look something like this:

This is pretty much the same code that was in the "before" version but the improved (refactored) version has it organized better in smaller, more related chunks of work. By writing composed methods, you break down the big problem into smaller, more manageable pieces.

The Game of Life, at a very high-level of abstraction, can be summarized as:

When you have this general "story" in front of you, you can systematically drill down to expand on the details of each step. Each time you drill down, you keep trying to stay as high-level abstract as you can until you get to a point where you can no longer avoid laying out the detailed instructions. This is how you manage complexity. For example, drilling down to "initialize board", you might describe it as:

You look at that again and decide whether you need to break each step down further or just translate it into detailed instructions.

Do you kind of see where this is going?
 
Stefan Evans
Bartender
Posts: 1837
10
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
One thing that makes this code hard to debug - you are dealing with random data each time.
There is a way to 'fix' this though.

Rather than using Math.random(), use a java.util.Random number generator.
The difference is that with the java.util.Random, you can then give it a 'starting seed'. Thereafter everytime you run the program it will give you the same pseudo random numbers. So your program will act the same each time.
Not what you want in real life, but great for debugging.

This code will generate a number between 1 and 20. The same number each time for the same starting seed.



Here is the change I suggest:



When you are done debugging, just change Random(1) to Random() and it will give you different numbers each time.

When I did that, and ran your program this is what I got for 5 cells:



Interesting output don't you think?
 
Aarden Axford
Greenhorn
Posts: 28
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Once again, the help is appreciated very much! I added some methods, now still working on debugging.... (this code is a bit messy because of it..)


I am having a LOT of issues trying to get the output correctly. It seems like the neighbours may not be incrementing properly, but I cannot figure out why...

 
Stefan Evans
Bartender
Posts: 1837
10
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you look at my the output from my running of the program, you can see that there is a batch of consecutive squares which are "alive", and no others

So neighbours has a value of 2 or 3 at those points.
Even when the original square has no neighbours.

What does that tell you?
Hint: Print out the value of "neighbours" at the time you do the checks on it.
What is it?
What do you think it should be?
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

You really should format your code better. It's less confusing when code is properly formatted and aligned.

These don't look like the standard B3/S23 rules (Born with exactly 3 neighbors, Survives with 2 or 3 neighbors). Is this what you're using or do you have a different set of rules?

You can simplify your conditional expressions by performing them in a certain order. Most of those playingBoard[i][j].equals(whatever) aren't really needed. For example, if a cell has less than 2 or more than 3 neighbors, the next generation will have an empty cell regardless of what the current generation has -- it either stays empty, dies of loneliness, or dies of overcrowding. Also, if a cell has exactly 3 neighbors, the next generation cell will be alive regardless of its current state because it will either survive or be born. In fact, I'm pretty sure you can do the whole die/born/no-change calculation with just this kind of structure:

This is without having to check the state of playingBoard[i][j] at all.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!