patterncsharpMinor
Reducing nested while statements for building a grid?
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):
I have also 3 "shape lists" that need to fill it in priority, as in:
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#:
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.
__ __ __
|__|__|__|
|__|__|__|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 typeThe 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.
The two inner
Now your code looks like:
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.