Gfrktyl

I have an assignment to draw rectangles that get larger and larger each time I draw them. Just like shown below. I've labeled them TL = Top Left, TR = Top Right, BL = Bottom Left, BR = Bottom Right. The smallest boxes are 8 pixels by 8 pixels. From this drawing I pulled these algorithms for each box. Where n represents the level you are drawing. Level 0 is the inside set of four and grows out by 1.

TL = (x-n(8), y - n(8))

TR = ((x + 8) - (n*8), y - n(8))

BL = (x - n(8), y - (n+1)(8))

BR = ((x+8)+(n*8), (y+8)-(n*8));

My real questions is this. I feel like I can combine these algorithms into something smaller but I'm new to this type of thinking. This is the first Computer Science Class I've taken that required me to do this type of thinking. Do you have any suggestions?

From these algorithms I wrote this code in Java.

    public static void drawRectangle(Graphics g, int xCenter, int yCenter, int levelsNeeded){

    //all squres are based off an 8 pixel system.  I use this to shift things around ALOT.

    final int CHANGECONSTANT = 8; 

    int width = 8;

    int height = 8;

    int dcg = 1; //Dimension Change Factor



    Point origin = new Point(xCenter -= CHANGECONSTANT, yCenter -= CHANGECONSTANT); //Sets Origin to top left of inner most set of squares



    for (int level = 0; level < levelsNeeded; level++)

    {

        g.setColor(Color.GRAY);

        //Top Left

        g.fillRect(

         (int)origin.getX()-(level*CHANGECONSTANT), // XCord

         (int)origin.getY()-(level*CHANGECONSTANT), // YCord

         width, //width. This stays at 8 for all iterations

         height*dcg);//height. This gets multiplied by odd factors each time

         //Draw outline

         g.setColor(Color.BLACK);

        g.drawRect(

         (int)origin.getX()-(level*CHANGECONSTANT), // XCord

         (int)origin.getY()-(level*CHANGECONSTANT), // YCord

         width, //width. This stays at 8 for all iterations

         height*dcg);//height. This gets multiplied by odd factors each time



        g.setColor(Color.GRAY);

        //Bottom Left

        g.fillRect(

         (int)origin.getX()-(level*CHANGECONSTANT), //XCrod

         (int)origin.getY()+((level+1)*CHANGECONSTANT), //YCord

         width*dcg, //width. This gets multiplied by odd factors each time 

         height); //height. This stays at 8 for all iterations

        g.setColor(Color.BLACK);

        //Draw Outline

        g.drawRect(

         (int)origin.getX()-(level*CHANGECONSTANT), //XCrod

         (int)origin.getY()+((level+1)*CHANGECONSTANT), //YCord

         width*dcg, //width. This gets multiplied by odd factors each time 

         height); //height. This stays at 8 for all iterations





        g.setColor(Color.WHITE);

        //Top Right

        g.fillRect(

         ((int)origin.getX()+CHANGECONSTANT)-(level*CHANGECONSTANT), //XCord

         (int)origin.getY()-(level*CHANGECONSTANT), //YCord

         width*dcg,//width. This gets multiplied by odd factors each time 

         height);//height. This stays at 8 for all iterations

         //Draw Outline

        g.setColor(Color.BLACK);

        g.drawRect(

         ((int)origin.getX()+CHANGECONSTANT)-(level*CHANGECONSTANT), //XCord

         (int)origin.getY()-(level*CHANGECONSTANT), //YCord

         width*dcg,//width. This gets multiplied by odd factors each time 

         height);//height. This stays at 8 for all iterations



        g.setColor(Color.WHITE);

        //Bottom Right

        g.fillRect(

         ((int)origin.getX()+CHANGECONSTANT)+(level*CHANGECONSTANT), //Xcord

         ((int)origin.getY()+CHANGECONSTANT)-(level*CHANGECONSTANT), //Ycord

         width,//width. This gets multiplied by odd factors each time 

         height*dcg);//Height. This stays at 8 for all iterations. 

        //Draw Outline

        g.setColor(Color.BLACK);

        g.drawRect(

         ((int)origin.getX()+CHANGECONSTANT)+(level*CHANGECONSTANT), //Xcord

         ((int)origin.getY()+CHANGECONSTANT)-(level*CHANGECONSTANT), //Ycord

         width,//width. This gets multiplied by odd factors each time 

         height*dcg);//Height. This stays at 8 for all iterations. 



        dcg += 2; //Increment DCG by two at end of each loop



    }

} 

Here is what my final product looks like right now. Disregard the triangle shapes that a different method that calls the first one multiple times.

This is my first quarter learning Java and I'm new to programming.

Naming

    public static void drawRectangle(Graphics g, int xCenter, int yCenter, int levelsNeeded){

I would call this drawRectangles, as it draws four rectangles on each iteration of the loop. Also, that leaves room for a drawRectangle method.

Typos

    //all squres are based off an 8 pixel system.  I use this to shift things around ALOT.

This has two typos.

    // all squares are based off an 8 pixel system.  I use this to shift things around A LOT.

Comments

    Point origin = new Point(xCenter -= CHANGECONSTANT, yCenter -= CHANGECONSTANT); //Sets Origin to top left of inner most set of squares

If you break this up into two lines, it's easier to read

    // Sets Origin to top left of inner most set of squares

    Point origin = new Point(xCenter -= CHANGECONSTANT, yCenter -= CHANGECONSTANT);

And it doesn't put a scroll bar on my StackExchange screen.

Refactoring

        g.setColor(Color.GRAY);

        //Top Left

        g.fillRect(

         (int)origin.getX()-(level*CHANGECONSTANT), // XCord

         (int)origin.getY()-(level*CHANGECONSTANT), // YCord

         width, //width. This stays at 8 for all iterations

         height*dcg);//height. This gets multiplied by odd factors each time

         //Draw outline

         g.setColor(Color.BLACK);

        g.drawRect(

         (int)origin.getX()-(level*CHANGECONSTANT), // XCord

         (int)origin.getY()-(level*CHANGECONSTANT), // YCord

         width, //width. This stays at 8 for all iterations

         height*dcg);//height. This gets multiplied by odd factors each time

You repeat this pattern four times. Consider

    public static void drawRectangle(Graphics g, int x, int y, int width, int height, Color color) {

        g.setColor(color);

        g.fillRect(x, y, width, height);



        g.setColor(Color.BLACK);

        g.drawRect(x, y, width, height);

    }

Then we can just write

    // the corner coordinates change by a constant amount each level

    final int maximumDelta = levelsNeeded * CHANGECONSTANT;

    for (int delta = 0; delta < maximumDelta; delta += CHANGECONSTANT)

    {

        final int left = (int)origin.getX() - delta;

        final int top = (int)origin.getY() - delta;



        // the width and height change twice as fast as the corner locations

        final int wide = narrow + 2 * delta;

        final int tall = small + 2 * delta;



        drawRectangle(g, left, top, narrow, tall, Color.GRAY);

        drawRectangle(g, left, bottom - delta, wide, small, Color.GRAY);

        drawRectangle(g, right - delta, top, wide, small, Color.WHITE);

        drawRectangle(g, right + delta, bottom + delta, narrow, tall, Color.WHITE);

    }

This assumes a previous change of width and height to narrow and small. We still have width and height in the helper method. Add right and bottom with them, outside the loop.

    final int right = (int)origin.getX() + CHANGECONSTANT;

    final int bottom = (int)origin.getY() + CHANGECONSTANT;

This is both shorter and more readable in my opinion. Rather than recalculating everything as needed, it calculates everything once. By using these names, we can easily see that a particular rectangle is the "left, top" that is drawn gray in "narrow, tall" aspect. We don't need comments, as these names are self-documenting.

We can make all of these final, but you don't need to do so.

I eliminated dcg. It was confusingly named and as I looked at it more carefully, unnecessary.

You could also make things easier by changing this from a static (class) method to an object method. Then your constructor could take a Graphics object. That would save passing it around all the time. You don't provide enough context to say whether this is a good idea. This might require moving this method to a different class.

I would add an underscore to CHANGECONSTANT and call it CHANGE_CONSTANT. Or change the name to something else entirely, e.g. INTERVAL or STANDARD_DIMENSION.

You haven't provided driver code; I haven't tried to run this version to see if it gives the same behavior. Beware of errors that might have been found by running the code.