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.
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
.