HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Reducing nested while statements for building a grid?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
whilegridstatementsreducingbuildingnestedfor

Problem

I have the following grid shape:

__ __ __
|__|__|__|
|__|__|__|


I have the following 3 types of shapes to fill it (they cannot rotate):

__ 
|__|    __
|__|   |__|   (6x6 one above, or full size)


I have also 3 "shape lists" that need to fill it in priority, as in:

ShapeList1 = all types
ShapeList2 = all types
ShapeList3 = only single type


The lists are actually stacks, and I cannot have any empty spaces. I also need to pop the the first shape off each stack, if it does not fill it, go to the next type. ShapeList3 will always be able to complete the grid as it only has the 1x1 type. If it gets to the end of ShapeList3 I try the shapes in order again, trying to fill the grid, until the shape is filled.

I have this working but I have a lot of while loops, including a nested loop. This is a code smell and I feel as if I could do this better. Here's my current building code in pseudo-C#:

var largeContentBlock = new LargeContentBlock();

    while (!largeContentBlock.Complete && aggregatePageFeed.ShapeList3.Any())
    {
    while (aggregatePageFeed.ShapeList1.Any()
                    && largeContentBlock.CanAddBlock(aggregatePageFeed.ShapeList1.Peek().Size)
                    && !largeContentBlock.Complete)
    {
        largeContentBlock.Add(aggregatePageFeed.ShapeList1.Pop());
    }

    while (aggregatePageFeed.ShapeList2.Any()
           && largeContentBlock.CanAddBlock(aggregatePageFeed.ShapeList2.Peek().Size)
           && !largeContentBlock.Complete)
    {
        largeContentBlock.Add(aggregatePageFeed.ShapeList2.Pop());
    }

        if (!largeContentBlock.Complete && largeContentBlock.CanAddBlock(aggregatePageFeed.ShapeList3.Peek().Size))
            largeContentBlock.Add(aggregatePageFeed.ShapeList3.Pop());
    }

    return largeContentBlock;


That's pretty ugly, I have it a little nicer by wrapping the while statements in functions. What pattern can I be using to make this nicer? I feel as if this is a solved problem.

Solution

There is a fair amount of common code that can be extracted into a function.

There is a common check in the while clause and the if clause.

boolean CanAdd(LargeContentBlock largeContentBlock, ShapeList shapes)
{
    return largeContentBlock.CanAddBlock(shapes.Peek().Size)
                && !largeContentBlock.Complete;
}


The two inner while loops are doing the same thing, just with two different lists. This is a perfect instance to abstracting the common code into a function.

void AddContent(LargeContentBlock largeContentBlock, ShapeList shapes)
{
    while (shapes.Any() && CanAdd(largeContentBlock, shapes))
    {
        largeContentBlock.Add(shapes.Pop());
    }
}


Now your code looks like:

var largeContentBlock = new LargeContentBlock();

while (!largeContentBlock.Complete && aggregatePageFeed.ShapeList3.Any())
{
    AddContent(largeContentBlock, aggregatePageFeed.ShapeList1);
    AddContent(largeContentBlock, aggregatePageFeed.ShapeList2);

    if (CanAdd(largeContentBlock, aggregatePageFeed.ShapeList3))
        largeContentBlock.Add(aggregatePageFeed.ShapeList3.Pop());
}

return largeContentBlock;

Code Snippets

boolean CanAdd(LargeContentBlock largeContentBlock, ShapeList shapes)
{
    return largeContentBlock.CanAddBlock(shapes.Peek().Size)
                && !largeContentBlock.Complete;
}
void AddContent(LargeContentBlock largeContentBlock, ShapeList shapes)
{
    while (shapes.Any() && CanAdd(largeContentBlock, shapes))
    {
        largeContentBlock.Add(shapes.Pop());
    }
}
var largeContentBlock = new LargeContentBlock();

while (!largeContentBlock.Complete && aggregatePageFeed.ShapeList3.Any())
{
    AddContent(largeContentBlock, aggregatePageFeed.ShapeList1);
    AddContent(largeContentBlock, aggregatePageFeed.ShapeList2);

    if (CanAdd(largeContentBlock, aggregatePageFeed.ShapeList3))
        largeContentBlock.Add(aggregatePageFeed.ShapeList3.Pop());
}

return largeContentBlock;

Context

StackExchange Code Review Q#39908, answer score: 7

Revisions (0)

No revisions yet.