Programming Assignment

[Phluxxx]

Thanks for your response I implemented a way to do it correctly based off my awesome friend's algorithm!

[mbozzi]

@Jonnin, do not loop on the end-of-file flag. Along with reading past the end of the file, the stream state may fail in ways that don't involve reaching the end-of-file. Instead, loop on the read operation itself, in this case std::getline().

Additionally, the length of an empty line is 0, not 1:

As you wrote, to ignore empty lines:
while(std::getline(stream, line)) rows += !!line.size();

[mbozzi]

It means that the linker couldn't find a particular symbol, and the crap that looks like line noise is the symbol name.

With some practice you can read those symbol names without any trouble. :D
In particular, it means that you said there is a function named operator<<() that prints out a cell, but the linker couldn't find it, either because:
a. there's no definition at all
b. the signature of the definition is inconsistent with the declaration.
c. you didn't link the right object code.

Since you're apparently using MSVC, the third option is unlikely. Check to make sure that you have defined the operator<<() for your Cell class.

[Phluxxx]

Damn you're a C++ god! haha thanks a ton for the in-depth explanation. My awesome friend gave me the solution though :D

[Phluxxx]

Hey mbozzi so my next step is to implement a rule where if a 'o' has more than 3 'o' that are horizontally, vertically, or diagonally adjacent to it, that particular cell will die, turning into '-'. Any tip on where to begin?

[mbozzi]

Yes.

Write a function that, given a particular spot on the board, returns the number of living adjacent cells.

Since you have a fixed size board, you'll have to figure out what to do about cells that are on the edge of that board, and handle those edge cases differently. I would suggest holding all the elements that are outside of the board at one particular state. Dead, maybe.

Something roughly like this:

template <typename T> 
constexpr bool in_halfopen(T const val, T const lower, T const upper) {
  return val >= lower && val < upper;
}

constexpr bool is_valid_board_space(int const x, int const y) {
  return in_halfopen(x, 0, board_rows) && 
    in_halfopen(y, 0, board_columns); 
}

bool cell_living(int const x, int const y) {
  if (! is_valid_board_space(x, y)) return false; 
  else return board[x][y].is_alive();
}

Then a function to return the number of living adjacent cells looks like:

int count_adjacent_living(int const x, int const y) {
  return cell_living(x - 1, y - 1) + 
         cell_living(x - 1, y)     + 
         cell_living(x - 1, y + 1) + /* and so on for each adjacent cell */
} 

[Phluxxx]

Hey mbozzi so my assignment restricts creating other functions and using other functions other than the ones that we have been given.

So this is kind of what I did but its kind of wrong

for (int i = 0; i < MAX_BOARD; i++)
			{
				for (int j = 0; j < MAX_BOARD; j++)
				{
					if (currentLife[i][j].getState() == true)
					{
						if (currentLife[i + 1][j + 1].getState() == true)
						{
							adjAlive++;
						}
						if (currentLife[i + 1][j].getState() == true)
						{
							adjAlive++;
						}
						if (currentLife[i + 1][j - 1].getState() == true)
						{
							adjAlive++;
						}
						if (currentLife[i][j - 1].getState() == true)
						{
							adjAlive++;
						}
						if (currentLife[i - 1][j - 1].getState() == true)
						{
							adjAlive++;
						}
						if (currentLife[i - 1][j].getState() == true)
						{
							adjAlive++;
						}
						if (currentLife[i - 1][j + 1].getState() == true)
						{
							adjAlive++;
						}
						if (currentLife[i][j + 1].getState() == true)
						{
							adjAlive++;
						}
					}
				}
			}

[mbozzi]

Hey mbozzi so my assignment restricts creating other functions and using other functions other than the ones that we have been given.

Usually restrictions like this apply only to the interface. Are you sure it's not acceptable to add private or internal functions available for the implementation's use only?

Restricting changes to the interface is a useful and realistic exercise, but restricting changes to the implementation is unrealistic -- decomposing problems into smaller ones is absolutely fundamental to handling complexity in software. It is critical to writing code that works correctly, is maintainable, and robust. A restriction against decomposing problems is actively destructive to your ability to solve problems and to your programming skill. Say it ain't so. Every design book I have ever read agrees with me.

Anyways:
0. You're in danger of running off the edge of the board. What happens if a live cell appears at index 0 on any axis while computing the number of adjacent living cells? The only way I can see this setup working is if you can guarantee that i and j will never be the last or first element of any of the arrays.

1. The number of adjacent cells never gets reset. You'll have to compute the number of adjacent cells for any cell, not just the living ones, and then compute the next state of any given cell depending on
a.) it's state
b.) the number of living adjacent cells.

Assuming that getState() return true, the large if-chain on lines [7, 38] can be shortened -- just sum the conditions. If there is a doubt that it returns a boolean (as there may be; it's poorly named), normalize them:

adjAlive = 
  !!currentLife[i + 1][j + 1].getState() + 
  !!currentLife[i + 1][j    ].getState() + 
  !!currentLife[i + 1][j - 1].getState() + 
  !!currentLife[i    ][j + 1].getState() + 
  !!currentLife[i    ][j - 1].getState() +
  !!currentLife[i - 1][j + 1].getState() + 
  !!currentLife[i - 1][j    ].getState() + 
  !!currentLife[i - 1][j - 1].getState();

Note: I couldn't work out whether or not there was a typo in the if-chain, so I just wrote it out.

The !! is double negation, a common-ish shortcut for converting something to a boolean. The trick is useful exactly in cases like this.

Psuedocode:

for (int i = 0; i < MAX_BOARD; ++i) 
  for (int j = 0; j < MAX_BOARD; ++j) {
    // reset me every iteration
    adjLiving = compute_adjacent_living_cells(); 
    Cell currentCell = currentLife[i][j];
    if (currentCell.getState()) { // if the current cell is alive 
      if (adjLiving <  2) currentCell.setState(dead); // die from underpopulation
      if (adjLiving == 3) currentCell.setState(live); // remain alive
      else currentCell.setState(dead); // die from overpopulation 
    } else if(adjLiving == 3) {
      currentCell.setState(live); // become alive from reproduction 
    }
    
    // now copy the adjusted cell to the next generation:
    nextLife[i][j] = currentCell;
  }
}

[Phluxxx]

So i don't understand your line 4. Where is the definition for compute_adjacent_living_cells(); ?

[mbozzi]

Line 4 is shorthand for

adjAlive = 
  !!currentLife[i + 1][j + 1].getState() + 
  !!currentLife[i + 1][j    ].getState() + 
  !!currentLife[i + 1][j - 1].getState() + 
  !!currentLife[i    ][j + 1].getState() + 
  !!currentLife[i    ][j - 1].getState() +
  !!currentLife[i - 1][j + 1].getState() + 
  !!currentLife[i - 1][j    ].getState() + 
  !!currentLife[i - 1][j - 1].getState();

Or an equivalent operation

[Phluxxx]

mbozzi so your code worked thanks a ton (:
I'm running into a bit of a problem though.

The part where we implement the rules of a cell and its life must be implemented into its own function, as I've done here.

bool CSCI2312::GameOfLife::executeRules(unsigned int countAlive, bool currentState)
{
	for (int i = 0; i < MAX_BOARD; i++)
	{
		for (int j = 0; j < MAX_BOARD; j++)
		{
			Cell currentCell = currentLife[i][j];
			if (currentCell.getState() == true)
			{
				if (countAlive < 2)
				{
					currentCell.setState(false);
				}
				if (countAlive == 3 || countAlive == 2)
				{
					currentCell.setState(true);
				}
				else
				{
					currentCell.setState(false);
				}
			}
			else if (countAlive == 3)
			{
				currentCell.setState(true);
			}
			nextLife[i][j] = currentCell;
		}
	}

	return false;
}

This function takes in "countAlive" as a parameter and what I was trying to do was pass in "adjLive" here so that the user can move on to the next "generation"/"life" as many times they want.

do
	{
		cout << "Would you like to move on to the next generation?" << endl;
		cout << "1: Move on" << endl;
		cout << "2: Return to the main menu" << endl;
		cin >> choice;
		cout << endl;

		switch (choice)
		{
		case 1:
			// Counts the amount of cells that are alive so that it is passed into "executeRules"
			for (int i = 0; i < MAX_BOARD; i++)
			{
				for (int j = 0; j < MAX_BOARD; j++)
				{
					int adjLive =
							currentLife[i + 1][j + 1].getState() +
							currentLife[i + 1][j].getState() +
							currentLife[i + 1][j - 1].getState() +
							currentLife[i][j + 1].getState() +
							currentLife[i][j - 1].getState() +
							currentLife[i - 1][j + 1].getState() +
							currentLife[i - 1][j].getState() +
							currentLife[i - 1][j - 1].getState();

			         }
		//	cout << endl << "The number of alive cells adjacent: " << adjLive << endl;
			executeRules(adjLive, true);
			nextGeneration();
			break;
		case 2:
			int main();
			break;
		default:
			cout << "You did not enter any number from 1 to 8. Please try again." << endl;
		}
	} while (choice != 2);

but I run into a problem where I can't access the variable within the nested for-loop. Any solutions?

[Mantorr22]

Peter Phluxxx : That is not what the function executeRules is for. Seems like you need people to guide you from the beginning to the end, instead of just giving you direction how to begin.

Phluxxx wrote:

Just looking for a head start or some sort of walkthrough in the beginning so that I can understand it fully

[Mantorr22]

@mbozzi

adjAlive = 
  !!currentLife[i + 1][j + 1].getState() + 
  !!currentLife[i + 1][j    ].getState() + 
  !!currentLife[i + 1][j - 1].getState() + 
  !!currentLife[i    ][j + 1].getState() + 
  !!currentLife[i    ][j - 1].getState() +
  !!currentLife[i - 1][j + 1].getState() + 
  !!currentLife[i - 1][j    ].getState() + 
  !!currentLife[i - 1][j - 1].getState();

Be careful not to cause any segmentation fault because +1 and -1 may be tricky.

[dhayden]

When iterating over the board, you shouldn't be using MAX_BOARD as the end point, you should use boardSize.

That executeRules() method is odd. I don't know why you'd pass in those two paramenters. On the other hand, if it was a method of the Cell() class, it would make perfect sense: it would determine the new state of a cell based on the old state and the number of living neighbors. Consider asking your prof if executeRules() should be in class Cell instead of GameOfLife. If it should be in GameOfLife then ask for an explanation of what it does.

To get the number of living neighbors, you can use a simple for loop. If i and j are the row and column of the cell whose nieghbors you want to check then:

int lowR = max(i-1, 0);
int highR = min(i+1, boardSize-1);
int lowC = max(j-1, 0);
int highC = min(j+1, boardSize-1);
int livingNeighbors=0;
for (r=lowR; r <= highR; ++r) {
    for (c=lowC; c<= highC; ++c) {
        if (r== i && c == j) continue;  // Don't count the current cell
        if (currentLife[r][c].getState()) ++livingNeighbors;
    }
}

All that stuff at the beginning is to ensure that you don't go out of bounds.

Checking for the current square each and every time in the inner loop is expensive. It's probably easier to just add yourself to the count in the loop and then adjust when you get out:

for (r=lowR; r <= highR; ++r) {
    for (c=lowC; c<= highC; ++c) {
        if (currentLife[r][c].getState()) ++livingNeighbors;
    }
}
if (currentLife[i][j].getState()) --livingNeighbors; // Don't count yourself 

The "advanced" part where you use the halo method lets you avoid all that min() and max() stuff by creating an extra set of squares around the board that can never have live cells. In that case you can use mbozzi's method of checking the 8 squares directly.