Counting Colors in Conway's Game of Life

I have a basic version of CGoL running with pdCurses. My goal was to have each newly spawned cell take on the dominant color of their neighbors (if a spawned cell is mostly surrounded by red, make it red). I managed to get a half-baked solution working, but it has a few problems, mainly:

• It requires another member vector to hold the frequency of colors

• It requires aforementioned vector to be marked as mutable so the constness of other functions isn't affected

• It required creating a struct to return duel results (the neighbor count, and the dominant color)

• The color frequency storage scheme is slightly confusing

If someone can think of a cleaner method of achieving this, I would appreciate it. I'll also take any other kind of critique you may have.

My main function to count the neighbors is:

NeighborData Population::getNeighborData(int x, int y, int depth) const {
    int count = 0;
    for (int cY = y - depth; cY = height) continue;
        for (int cX = x - depth; cX = width || (cX == x && cY == y)) continue;

            unsigned char color = getPointColor(cX, cY);

            if (color != '\0') {
                count += 1;
                colorFreqs[color] += 1;
            }

        }
    }
    unsigned char c = consumeColorFrequencies();
    return NeighborData(count,c);
}

• vector colorFreqs has a pre-allocated slot for each color (only 16 on my machine). Every time we check a color, we look up the color using the color as an index, and increment its count.

• consumeColorFrequenices() is the main function that I'm asking about. It "consumes" the frequency vector; returning the dominant color (or the first found color if more the one have an equal frequency)

• NeighborData is a small struct with 2 members: the count, and the dominant color. I needed a way to return both bits of data at once to my decideLifeOf() method.

consumeColorFrequencies():

```
unsigned char Population::consumeColorFrequencies() const {
int hIn

I don't think there is anything wrong with your general approach (or at least I don't have a better suggestion).

On an implementation level I've a few suggestions

• As mentioned before, I'd replace the class member mutable std::vector colorFreqs; with a local std::array colorFreqs{}; in getNeighborData and pass the array as a const ref parameter to consumeColorFrequencies. This gets rid of the mutable problem and might even increase performance.

-
I'd write the getNeighborData function a little different:

NeighborData Population::getNeighborData(int x, int y, int depth) const {
    std::array colorFreqs{};
    int count = 0;
    for (int cY = std::max(0, y - depth); cY <= std::min(height-1, y + depth); cY++) {      
        for (int cX = std::max(0, x - depth); cX <= std::min(width-1, x + depth); cX++) {
            if (cX == x && cY == y) continue;
            unsigned char color = getPointColor(cX, cY);
            if (color != '\0') {
                count++;
                colorFreqs[color]++;
            }
        }
    }
    unsigned char c = consumeColorFrequencies(colorFreqs);
    return NeighborData(count, c);
}

Whether that is easier to understand than your version is up for discussion, but it should be a little more efficient.

-
consumeColorFrequenciescan be simplified by using an STL algorithm:

unsigned char Population::consumeColorFrequencies(const std::array& colorFreqs) const {
    auto it = std::max_element(std::begin(colorFreqs), std::end(colorFreqs));
    return std::distance(std::begin(colorFreqs),it);
}

-
In response to the comment about multithreading: You can (more or less) trivially parallelize compileOutput by letting each thread generate the new cells for a slice of the world (e.g. a quarter of the rows on a 4-Core machine). There are many parallel loop implementations out there that can make that Task even easier. Obviously this is only sensible for very large grids.

StackExchange Code Review Q#86402, answer score: 4