patterncsharpMinor
Find the center of n squares which together build a rectangle
Viewed 0 times
therectangletogethersquarescenterfindwhichbuild
Problem
I have a rectangle with size
Now the code works, but I think it's a little bit ugly. Any hints on how I can improve it?
w and height h. Now I want to split this rectangle into n new rectangles that are as similar as possible to a square. Afterwards I'd like to calculated the center of each square.public static List getCenters(int number, double width, double height)
{
List points = new List();
int originalNumberOfSquares = number;
int numberOfSquares = number;
if (numberOfSquares % 2 == 1)
{
numberOfSquares++;
}
int rectangleWidth = Convert.ToInt32(width);
int rectangleHeight = Convert.ToInt32(height);
double minDistance = Double.MaxValue;
int nSquaresInRow = -1;
int nSquaresInColumn = -1;
for (int i = 0; i rectangleWidth)
{
xSquareCenter = xSquareCenter - rectangleWidth;
}
int row = (r / nSquaresInColumn) + 1;
int ySquareCenter = ((2 * row) - 1) * (squareHeight / 2);
points.Add(new Point(Convert.ToDouble(xSquareCenter), Convert.ToDouble(ySquareCenter)));
}
return points;
}Now the code works, but I think it's a little bit ugly. Any hints on how I can improve it?
Solution
A couple of notes:
- The width, height and number of sub-rectangles should be parameters, not local variables. If you should, for example, want to print out the centres for differing values of
w,handn, calling the method in a loop with different arguments is more convenient than changing the code and rerunning it multiple times.
- Determining the size of the sub-rectangles and collecting their centres could be done in two separate methods because a) they can easily be separated as they're not intertwined b) it makes it immediately obvious which part is done by which code and c) they might be useful on their own.
- Rather than outputting the centres, you should return them in an array or list. This way you can also easily use the method in a context where you don't want to print the centres or want to do something to them before printing. Also it's generally a good practice to separate IO code from logic code.
- Since both
iandjare ints and thusminDistancewill only ever hold integer values, it should have typeint, notdouble. Otherwise it leaves the impression that it could possibly have a non-integer value.
- On a similar note you should probably make the type of
widthandheightintas well since the first thing you do is to truncate them toints. Having their type bedoublemight make the user of your method think the results will be more accurate than they really are.
Context
StackExchange Code Review Q#702, answer score: 5
Revisions (0)
No revisions yet.