Search code examples
c#refactoringcode-cleanup

How can I refactor these two functions with repeating code?


I'm trying to refactor my code for higher readability and I'm new to this.

I have two functions called UpFirst() and RightFirst() and they include common lines of code and I want to clean the repeating code. Could you show me a good way to clean them from repeating lines?

private bool UpFirst()
{
    var sameBlocks = new List<Tiles>();
    var isAnyMatch = false;

    for (var x = 0; x < _RowCount; x++)
    {
        for (var y = 0; y < _ColumnCount; y++)
        {
            if (y > _ColumnCount - _MinNumberToBlast)
                continue;

            var currentBlock = _tiles[x, y];

            var isAllMatch = true;
            for (var i = 1; i < _MinNumberToBlast; i++)
            {
                var blockToCheck = _tiles[x, y + i];

                var isMatch = currentBlock.Color == blockToCheck.Color;
                isAllMatch = isMatch;
                if (!isMatch) break;
            }

            if (isAllMatch)
            {
                isAnyMatch = true;
                sameBlocks.Add(currentBlock);

                var newBlocks = FloodFill(x, y);

                foreach (var block in newBlocks.Where(block => !sameBlocks.Contains(block)))
                {
                    sameBlocks.Add(block);
                }

                var number = sameBlocks.Count;
                var matchType = TileGroupType.Default;

                if (number > _ConditionACount)
                {
                    matchType = TileGroupType.A;

                    if (number > _ConditionBCount)
                    {
                        matchType = TileGroupType.B;

                        if (number > _ConditionCCount)
                        {
                            matchType = TileGroupType.C;
                        }
                    }
                }

                foreach (var block in sameBlocks)
                {
                    block.SetMatchGroupType(matchType);
                }
            }

            sameBlocks.Clear();
        }
    }

    return isAnyMatch;
}
private bool RightFirst()
{
    var sameBlocks = new List<Tiles>();
    var isAnyMatch = false;

    for (var y = 0; y < _ColumnCount; y++)
    {
        for (var x = 0; x < _RowCount; x++)
        {
            if (x > _RowCount - _MinNumberToBlast)
                continue;

            var currentBlock = _tiles[x, y];
            currentBlock.SetMatchGroupType(TileGroupType.Default);

            var isAllMatch = true;
            for (var i = 1; i < _MinNumberToBlast; i++)
            {
                var blockToCheck = _tiles[x + i, y];

                var isMatch = currentBlock.Color == blockToCheck.Color;
                isAllMatch = isMatch;
                if (!isMatch) break;
            }

            if (isAllMatch)
            {
                isAnyMatch = true;
                sameBlocks.Add(currentBlock);

                var newBlocks = FloodFill(x, y);

                foreach (var block in newBlocks.Where(block => !sameBlocks.Contains(block)))
                {
                    sameBlocks.Add(block);
                }

                var number = sameBlocks.Count;
                var matchType = TileGroupType.Default;

                if (number > _ConditionACount)
                {
                    matchType = TileGroupType.A;

                    if (number > _ConditionBCount)
                    {
                        matchType = TileGroupType.B;

                        if (number > _ConditionCCount)
                        {
                            matchType = TileGroupType.C;
                        }
                    }
                }

                foreach (var block in sameBlocks)
                {
                    block.SetMatchGroupType(matchType);
                }
            }

            sameBlocks.Clear();
        }
    }

    return isAnyMatch;
}

thank you!

I tried creating a third function including the if(isAllMatch) block and call it from both of them but didn't work because I had to define x and y and I could not handle it.


Solution

  • You could add a "neutral" method having parameters for the first and second count and a delegate to get the coordinates in the right order

    private bool ProcessTiles(int count1, int count2,
                              Func<int, int, (int, int)> getXY)
    {
        ...
        for (int i = 0; i < count1; i++)
        {
            for (int j = 0; j < count2; j++)
            {
                if (j > count2 - _MinNumberToBlast)
                    continue;
    
                var (x, y) = getXY(i, j);
    
                Tiles currentBlock = _tiles[x, y];
    
                bool isAllMatch = true;
                for (int k = 1; k < _MinNumberToBlast; k++)
                {
                    var (dx, dy) = getXY(0, k);
                    var blockToCheck = _tiles[x + dx, y + dy];
                    ...
                }
                ...
                var newBlocks = FloodFill(x, y);
                ...
            }
        }
    }
    
    private bool UpFirst()
    {
        ProcessTiles(_RowCount, _ColumnCount, (i, j) => (i, j));
    }
    
    private bool RightFirst()
    {
        ProcessTiles(_ColumnCount, _RowCount, (i, j) => (j, i));
    }
    

    Func<int, int, (int, int)> getXY is a function that takes a pair of indexes and returns a tuple with those indexes. UpFirst passes the lambda expression (i, j) => (i, j) to return the indexes in the same order, whereas RightFirst uses (i, j) => (j, i) to invert the indexes.

    var (x, y) = getXY(i, j); then gets the the right x, y values from the neutral indexes. Later we get the index added to either x or y with

    var (dx, dy) = getXY(0, k);
    var blockToCheck = _tiles[x + dx, y + dy];
    

    In UpFirst we will get dx = 0, dy = k and in RightFirst we will get dx = k, dy = 0.