Grouping rectangles horizontally and vertically

Question

As you can see the below code for each method is that same, except for the properties it uses. For example 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<Rectangle[]> GroupRectanglesHorizontally(Rectangle[] rectangles, out int averageGap)
{
    List<int> nlGaps = new List<int>();
    List<Rectangle[]> rectangleGroups = new List<Rectangle[]>();
    for (int bi = 0, bc = rectangles.Length; bi < bc; ++bi)
    {
        if (rectangleGroups.Any(g => 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) <= nTolerance
                                                            && b.X > rectangles[bi].X)
                                                    .ToArray();
        if (baSimiliarHeight.Length <= 1)
        {
            rectangleGroups.Add(new Rectangle[] { rectangles[bi] });
            continue;
        }
        baSimiliarHeight = baSimiliarHeight.OrderBy(b => 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<Rectangle> rectangleList = new List<Rectangle>();
        rectangleList.Add(rectangles[bi]);
        rectangleList.Add(baSimiliarHeight[0]);

        for (int sbi = 1; sbi < baSimiliarHeight.Length; ++sbi)
        {
            int nGap = baSimiliarHeight[sbi].X - (baSimiliarHeight[sbi - 1].X + baSimiliarHeight[sbi - 1].Width);
            if (nGap <= nInitialGap + nInitialGapTolerance && nGap >= 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<Rectangle>(new Rectangle[] { rectangles[bi] });
                break;
            }
        }

        rectangleGroups.Add(rectangleList.ToArray());
    }

    averageGap = (int)nlGaps.Average();
    return rectangleGroups;
}

private List<Rectangle[]> GroupRectanglesVertically(Rectangle[] rectangles, out int averageGap)
{
    List<int> nlGaps = new List<int>();
    List<Rectangle[]> rectangleGroups = new List<Rectangle[]>();
    for (int bi = 0, bc = rectangles.Length; bi < bc; ++bi)
    {
        if (rectangleGroups.Any(g => g.Contains(rectangles[bi])))
        {
            continue;
        }

        int nTolerance = (int)Math.Ceiling(rectangles[bi].Width * BOX_LINEUP_TOLERANCE);
        Rectangle[] baSimiliarWidth = rectangles.Where(b => Math.Abs(b.X - rectangles[bi].X) <= nTolerance
                                                            && b.Y > rectangles[bi].Y)
                                                .ToArray();
        if (baSimiliarWidth.Length <= 1)
        {
            rectangleGroups.Add(new Rectangle[] { rectangles[bi] });
            continue;
        }
        baSimiliarWidth = baSimiliarWidth.OrderBy(b => b.Y).ToArray();

        int nInitialGap = baSimiliarWidth[0].Y - (rectangles[bi].Y + rectangles[bi].Height);
        int nInitialGapTolerance = (int)Math.Ceiling(nInitialGap * BOX_LINEUP_TOLERANCE);
        nlGaps.Add(nInitialGap);

        List<Rectangle> rectangleList = new List<Rectangle>();
        rectangleList.Add(rectangles[bi]);
        rectangleList.Add(baSimiliarWidth[0]);

        for (int sbi = 1; sbi < baSimiliarWidth.Length; ++sbi)
        {
            int nGap = baSimiliarWidth[sbi].Y - (baSimiliarWidth[sbi - 1].Y + baSimiliarWidth[sbi - 1].Height);
            if (nGap <= nInitialGap + nInitialGapTolerance && nGap >= nInitialGap - nInitialGapTolerance)
            {
                rectangleList.Add(baSimiliarWidth[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<Rectangle>(new Rectangle[] { rectangles[bi] });
                break;
            }
        }

        rectangleGroups.Add(rectangleList.ToArray());
    }

    averageGap = (int)nlGaps.Average();
    return rectangleGroups;
}

I will also appreciate general feedback about the code.

Answer 1

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:

private List<Rectangle[]> 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<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);
}

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) <= nTolerance
                                                        && b.X > 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.

Answer 2

To keep your code clean, you could define delegates:

delegate int AxisLocationDelegate(Rectangle r, bool primaryAxis);
delegate int AxisSizeDelegate(Rectangle r, bool primaryAxis);

Then define a unified GroupRectanglesBy function that takes an implementation of each of these delegates as parameters:

private List<Rectangle[]> GroupRectanglesBy(Rectangle[] rectangles,
    AxisLocationDelegate getLocation, AxisSizeDelegate getSize, out int averageGap)
{
    // ...

Here in the main function you could refer to the delegate functions getLocation and getSize and specify whether you're interested in the primary or secondary axis, e.g.:

    int nTolerance = (int)Math.Ceiling(getSize(rectangles[bi], false) * BOX_LINEUP_TOLERANCE);
    Rectangle[] baSimiliarMagnitude = rectangles.Where(
        b => Math.Abs(getLocation(b, false) - getSize(rectangles[bi], false)) <= nTolerance
                && getLocation(b, true) > getLocation(rectangles[bi], true))
        .ToArray();

Then call with horizontal/vertical implementations of the delegates that pick out the relevant properties:

private List<Rectangle[]> GroupRectanglesHorizontally(Rectangle[] rectangles, out int averageGap)
{
    return GroupRectanglesBy(
        rectangles,
        (rect, primaryAxis) => primaryAxis ? rect.Location.X : rect.Location.Y,
        (rect, primaryAxis) => primaryAxis ? rect.Width : rect.Height,
        out averageGap);
}

private List<Rectangle[]> GroupRectanglesVertically(Rectangle[] rectangles, out int averageGap)
{
    return GroupRectanglesBy(
        rectangles,
        (rect, primaryAxis) => primaryAxis ? rect.Location.Y : rect.Location.X,
        (rect, primaryAxis) => primaryAxis ? rect.Height : rect.Width,
        out averageGap);
}

Edit: if you were feeling particularly evil and wanted to use the boolean XOR operator ^, you could even reduce this to one function:

private List<Rectangle[]> GroupRectangles(Rectangle[] rectangles, bool vertically, out int averageGap)
{
    return GroupRectanglesBy(
        rectangles,
        (rect, primaryAxis) => primaryAxis ^ vertically ? rect.Location.X : rect.Location.Y,
        (rect, primaryAxis) => primaryAxis ^ vertically ? rect.Width : rect.Height,
        out averageGap);
}