patternjavaMinor
Positioning boxes in a grid
Viewed 0 times
boxespositioninggrid
Problem
A small greedy algorithm to position boxes in a grid.
Version A
Version B
Version A has the advantage of fewer variables but has to make sure to check for the first case when the
Which one would be better practice? Any suggestions for improvement?
(and I should add that concurrency is not an issue in this case.)
Version A
int lCurrentIndex = 0;
BoxInchesInterface lPreviousBbox = lSortedBboxes.iterator().next();
double lEndPositionForCurrentIndex = pDimension.getEnd(lPreviousBbox);
for (BoxInchesInterface lBbox : lSortedBboxes)
{
boolean lIsFirstIteration = lBbox == lPreviousBbox;
if (!lIsFirstIteration && pDimension.getStart(lBbox) >= lEndPositionForCurrentIndex) {
lCurrentIndex += 1;
lEndPositionForCurrentIndex = pDimension.getEnd(lBbox);
} else if (pDimension.getEnd(lBbox) < lEndPositionForCurrentIndex) {
lEndPositionForCurrentIndex = pDimension.getEnd(lBbox);
}
lIndexFromBbox.put(lBbox, lCurrentIndex);
lPreviousBbox = lBbox;
}
return lIndexFromBbox;Version B
int lCurrentIndex = 0;
Iterator lItor = lSortedBboxes.iterator();
BoxInchesInterface lPreviousBbox = lItor.next();
double lEndPositionForCurrentIndex = pDimension.getEnd(lPreviousBbox);
lIndexFromBbox.put(lPreviousBbox, lCurrentIndex);
while (lItor.hasNext())
{
BoxInchesInterface lBbox = lItor.next();
if (pDimension.getStart(lBbox) >= lEndPositionForCurrentIndex)
{
lCurrentIndex += 1;
lEndPositionForCurrentIndex = pDimension.getEnd(lBbox);
} else if (pDimension.getEnd(lBbox) < lEndPositionForCurrentIndex) {
lEndPositionForCurrentIndex = pDimension.getEnd(lBbox);
}
lIndexFromBbox.put(lBbox, lCurrentIndex);
lPreviousBbox = lBbox;
}Version A has the advantage of fewer variables but has to make sure to check for the first case when the
lBbox is equal to lPreviousBox. Version B has the advantage that lPreviousBox is always different from lBbox, but has to deal with the iterator.Which one would be better practice? Any suggestions for improvement?
(and I should add that concurrency is not an issue in this case.)
Solution
Version A is definitely better, as it employs a more familiar pattern to get the job done, and it makes use of bit of trivial code (
I would trivialize the trivial code even more, by saying
I could perhaps give more advise if you explained what you mean by 'algorithm to position boxes in a grid'. A grid is generally thought of as a two-dimensional structure, but I only see operations in one dimension, on variables which are defined outside of the code fragment that you provided, so... what is this code trying to accomplish?
boolean lIsFirstIteration = lBbox == lPreviousBbox) to save you from duplication of non-trivial code. (lIndexFromBbox.put(lPreviousBbox, lCurrentIndex))I would trivialize the trivial code even more, by saying
bool IsFirstIteration = true right before entering the loop, and IsFirstIteration = false as the last instruction of the loop. Note that even though this is two lines of code instead of one, its complexity is smaller, because it deals with constants, not with variables.I could perhaps give more advise if you explained what you mean by 'algorithm to position boxes in a grid'. A grid is generally thought of as a two-dimensional structure, but I only see operations in one dimension, on variables which are defined outside of the code fragment that you provided, so... what is this code trying to accomplish?
Context
StackExchange Code Review Q#6354, answer score: 3
Revisions (0)
No revisions yet.