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

Find the center of n squares which together build a rectangle

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

Problem

I have a rectangle with size 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, h and n, 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 i and j are ints and thus minDistance will only ever hold integer values, it should have type int, not double. 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 width and height int as well since the first thing you do is to truncate them to ints. Having their type be double might 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.