Removing duplication in horizontal and vertical checks

// Check horizontals
for (int y = 0; y GetShape();
        if (shape == groupShape)
        {
            groupSize++;
            if (groupSize == m_MatchSize)
            {
                g_Audio->PlaySound(m_GemDestroyedSound.c_str());
                m_Score += shape->GetScore();
                for (int tx = x - m_MatchSize + 1; tx GetShape();
        if (shape == groupShape)
        {
            groupSize++;
            if (groupSize == m_MatchSize)
            {
                g_Audio->PlaySound(m_GemDestroyedSound.c_str());
                m_Score += shape->GetScore();
                for (int ty = y - m_MatchSize + 1; ty <= y; ty++)
                {
                    RANDOMIZE(GetItem(x, ty))
                }
                return;
            }
        }
        else
        {
            groupSize = 1;
            groupShape = shape;
        }
    }
}

Is there an easy way to clean this up? It seems like a lot of code duplication.

I have no qualms about using #defines, and I cannot use C++11.

The most straightforward way to do this is to introduce a new boolean variable horz that is true in the case that you wish to check horizontals. With that simple addition, both routines now can be expressed as one:

// if (horz) is true, check horizontals, else check verticals
aLimit = horz ? sGridHeight : sGridWidth;
bLimit = horz ? sGridWidth : sGridHeight;
for (int a = 0; a GetShape();  
        if (shape == groupShape)
        {
            groupSize++;
            if (groupSize == m_MatchSize)
            {
                g_Audio->PlaySound(m_GemDestroyedSound.c_str());
                m_Score += shape->GetScore();
                for (int tb = b - m_MatchSize + 1; tb <= b; tb++)  
                {
                    RANDOMIZE(horz ? GetItem(tb, a) : GetItem(a, tb));
                }
                return;
            }
        }
        else
        {
            groupSize = 1;
            groupShape = shape;
        }
    }
}

However, there are a number of things that suggest opportunities for further improvement. In particular, the use of pointers is probably not a good idea. For example, these two lines:

GridItem* gridItem = (horz ? GetItem(b, a) : GetItem(a, b));
    Shape* shape = gridItem->GetShape();

Are the only place that the gridItem pointer is used, and it's not checked for nullptr before it's dereferenced. So either that's a bug if GetItem() could possibly return a nullptr or it should instead be written as a reference. Even better, it could be combined into a single line and the variable eliminated completely.

Using this modified code is OK, but it's still a little unwieldy, so I would suggest that the GetItem() function should take a third argument which is the bool horz. That would make those same two lines of code look like this:

Shape* shape = GetItem(a, b, horz).GetShape();

Much nicer, but again, I'd turn this pointer into a reference (or better yet, a const reference if possible).

StackExchange Code Review Q#49255, answer score: 3