patterncsharpMinor
Grouping rectangles horizontally and vertically
Viewed 0 times
rectanglesgroupinghorizontallyverticallyand
Problem
As you can see the below code for each method is that same, except for the properties it uses. For example
Notes:
The all caps constant is
The two methods:
```
private List GroupRectanglesHorizontally(Rectangle[] rectangles, out int averageGap)
{
List nlGaps = new List();
List rectangleGroups = new List();
for (int bi = 0, bc = rectangles.Length; bi g.Contains(rectangles[bi])))
{
continue;
}
int nTolerance = (int)Math.Ceiling(rectangles[bi].Height * BOX_LINEUP_TOLERANCE);
Rectangle[] baSimiliarHeight = rectangles.Where(b => Math.Abs(b.Y - rectangles[bi].Y) rectangles[bi].X)
.ToArray();
if (baSimiliarHeight.Length b.X).ToArray();
int nInitialGap = baSimiliarHeight[0].X - (rectangles[bi].X + rectangles[bi].Width);
int nInitialGapTolerance = (int)Math.Ceiling(nInitialGap * BOX_LINEUP_TOLERANCE);
nlGaps.Add(nInitialGap);
List rectangleList = new List();
rectangleList.Add(rectangles[bi]);
rectangleList.Add(baSimiliarHeight[0]);
for (int sbi = 1; sbi = nInitialGap - nInitialGapTolerance)
{
rectangleList.Add(baSimiliarHeight[sbi]);
nlGaps.Add(nGap);
}
else if (nGap > nInitialGap)
{
break;
}
else
{
//Initial gap was larger so first rectangle is in its own group
rectangleList = new List(new Rectangle[] { rectangles[bi] });
break;
}
}
rectangleGroups.Add(rectangleList.ToArray());
}
averageGap = (int)nlGaps.Average();
return rectangleGroups;
}
pri
X vs Y and Width vs Height. Is there anyway that I can refactor the code to use a common main method that changes on which property it uses?Notes:
The all caps constant is
BOX_LINEUP_TOLERANCE has a value of 0.15f and is all caps due to a company policy.The two methods:
```
private List GroupRectanglesHorizontally(Rectangle[] rectangles, out int averageGap)
{
List nlGaps = new List();
List rectangleGroups = new List();
for (int bi = 0, bc = rectangles.Length; bi g.Contains(rectangles[bi])))
{
continue;
}
int nTolerance = (int)Math.Ceiling(rectangles[bi].Height * BOX_LINEUP_TOLERANCE);
Rectangle[] baSimiliarHeight = rectangles.Where(b => Math.Abs(b.Y - rectangles[bi].Y) rectangles[bi].X)
.ToArray();
if (baSimiliarHeight.Length b.X).ToArray();
int nInitialGap = baSimiliarHeight[0].X - (rectangles[bi].X + rectangles[bi].Width);
int nInitialGapTolerance = (int)Math.Ceiling(nInitialGap * BOX_LINEUP_TOLERANCE);
nlGaps.Add(nInitialGap);
List rectangleList = new List();
rectangleList.Add(rectangles[bi]);
rectangleList.Add(baSimiliarHeight[0]);
for (int sbi = 1; sbi = nInitialGap - nInitialGapTolerance)
{
rectangleList.Add(baSimiliarHeight[sbi]);
nlGaps.Add(nGap);
}
else if (nGap > nInitialGap)
{
break;
}
else
{
//Initial gap was larger so first rectangle is in its own group
rectangleList = new List(new Rectangle[] { rectangles[bi] });
break;
}
}
rectangleGroups.Add(rectangleList.ToArray());
}
averageGap = (int)nlGaps.Average();
return rectangleGroups;
}
pri
Solution
You can have a boolean as a parameter and select X or Y and Width or Height based on that parameter. You can declare the method like this:
You can use the code from the horizontal method and modify the property calls to use the boolean. For example, you would replace
It can then be used in your previous methods:
Alternatively, you could have functions as parameters for selecting the properties, like using a lambda expression in
Edit: as for the first method itself:
In the following line of code:
it won't get rectangles that are at the exact same X position. But that won't matter if it's guaranteed that none of the rectangles overlap. Then there's this code right after:
Because rectangles with the same X position weren't matched before,
In the inner
If you get a list of the rectangles ordered by their X position right away and loop through that instead of the original list, you can take some shortcuts in the body of the
Edit 2:
You should add more descriptive names. I don't know what
private List GroupRectangles(Rectangle[] rectangles, bool horizontally, out int averageGap)You can use the code from the horizontal method and modify the property calls to use the boolean. For example, you would replace
rectangle.X with horizontally ? rectangle.X : rectangle.Y and rectangle.Width with horizontally ? rectangle.Width : rectangle.Height.It can then be used in your previous methods:
private List GroupRectanglesHorizontally(Rectangle[] rectangles, out int averageGap)
{
return GroupRectangles(rectangles, true, out averageGap);
}
private List GroupRectanglesVertically(Rectangle[] rectangles, out int averageGap)
{
return GroupRectangles(rectangles, false, out averageGap);
}Alternatively, you could have functions as parameters for selecting the properties, like using a lambda expression in
OrderBy().Edit: as for the first method itself:
In the following line of code:
Rectangle[] baSimiliarHeight = rectangles.Where(b => Math.Abs(b.Y - rectangles[bi].Y) rectangles[bi].X)
.ToArray();it won't get rectangles that are at the exact same X position. But that won't matter if it's guaranteed that none of the rectangles overlap. Then there's this code right after:
if (baSimiliarHeight.Length <= 1)
{
rectangleGroups.Add(new Rectangle[] { rectangles[bi] });
continue;
}Because rectangles with the same X position weren't matched before,
baSimiliarHeight won't contain rectangles[bi], so if it has a length of 1, that 1 element will be a different rectangle, but only the original rectangle will get added as a group.In the inner
for loop, the body of the last else only adds the initial rectangle, but doesn't do anything about the previously added nGaps, so it's possible for the loop to add a bunch of nGaps but only add the initial rectangle.If you get a list of the rectangles ordered by their X position right away and loop through that instead of the original list, you can take some shortcuts in the body of the
for loop.Edit 2:
You should add more descriptive names. I don't know what
bi, bc, the nl in nlGaps, etc. stand for. Short names are okay for lambda expressions and loop variables, but even then you should stick to x, y, i, j, or a letter that relates to the object. I think it would make more sense to use r for a rectangle than b.Code Snippets
private List<Rectangle[]> GroupRectangles(Rectangle[] rectangles, bool horizontally, out int averageGap)private List<Rectangle[]> GroupRectanglesHorizontally(Rectangle[] rectangles, out int averageGap)
{
return GroupRectangles(rectangles, true, out averageGap);
}
private List<Rectangle[]> GroupRectanglesVertically(Rectangle[] rectangles, out int averageGap)
{
return GroupRectangles(rectangles, false, out averageGap);
}Rectangle[] baSimiliarHeight = rectangles.Where(b => Math.Abs(b.Y - rectangles[bi].Y) <= nTolerance
&& b.X > rectangles[bi].X)
.ToArray();if (baSimiliarHeight.Length <= 1)
{
rectangleGroups.Add(new Rectangle[] { rectangles[bi] });
continue;
}Context
StackExchange Code Review Q#132203, answer score: 4
Revisions (0)
No revisions yet.