Java Drawing House

Question

Can anybody tell me if there is a better way code this? As an exercise I was supposed to draw a simple house, and this is the main body of the code I have only I feel it could be way better but can't see anyway to make it so.

The code works as it should, I just as I said want to make it better and more efficient.

public class HouseCreator
{
    private double x;
    private double y;
    private double measurement;

    public HouseCreator(double x, double y, double measurement)
    {
        this.x = x;
        this.y = y;
        this.measurement = measurement;
    }

    public void draw(Graphics2D g2)
    {
        //Roof points
        Point2D.Double p1 = new Point2D.Double(x, y - (measurement / 2));   //Top point roof
        Point2D.Double p2 = new Point2D.Double(x - (measurement / 2), y);   //Bottom left roof
        Point2D.Double p3 = new Point2D.Double(x + (measurement / 2), y);   //Bottom right roof

        //Roof lines
        Line2D.Double r1 = new Line2D.Double(p1, p2);   //Left side roof
        Line2D.Double r2 = new Line2D.Double(p1, p3);   //Right side roof

        //House starting coordinates and shape
        int xCoord = (int)p2.getX();
        int yCoord = (int)p2.getY();

        Rectangle house = new Rectangle(xCoord, yCoord, (int)measurement, (int)measurement);

        //Door starting coordinates and shape
        xCoord = (int)(p2.getX() + (measurement / 5));
        yCoord = (int)(p2.getY() + (measurement / 2));

        Rectangle door = new Rectangle(xCoord, yCoord,(int)(measurement / 4), (int)(measurement / 2)); //Door shape

        //Window starting coordinates and shape
        xCoord = xCoord + (int)(measurement / 2.5);
        yCoord = yCoord + (int)(measurement / 10);

        Rectangle window = new Rectangle(xCoord, yCoord, (int)(measurement / 4), (int)(measurement / 4)); //Windows shape

        g2.draw(r1);
        g2.draw(r2);
        g2.draw(house);
        g2.draw(door);
        g2.draw(window);

    }
}

Answer 1

Variables and data types

As you calculate the result of measurement / 2 pretty often, I would store it in a variable as it is a bit, not really noticeable faster but it makes the code more readable in my opinion. Just create a variable at the beginning of the method that stores the result of the calculation:

int halfMeasurement = measurement / 2;

This has another advantage: You don't need to cast to an int when referring to this variable as it is already an integer.

If you don't need the variable measurement to be a double I would replace it with an int as casting would then be nearly entirely redundant:

public class HouseCreator {

    // Other variables (x, y) can still be doubles
    private int measurement;

    public HouseCreator(double x, double y, int measurement) {
        // Initialize variables here
    }

    // Rest of the class...
}

Redundant variables and naming

In the method #draw(g2: Graphics2D) you have some variables that are only accessed once and don't have to be variables at all. Still, I would rename the parameter to have the name graphics as g2 is not as meaningful. If you still want those to be variables for readability I would suggest to give them better names that are easier to understand.

The variables r1 and r2 are some good examples for this. As mentioned you now have to options, either rename them to have an informative name or not to use them at all and rather use the values of those directly when drawing the lines:

Renaming

Line2D.Double leftRoof = new Line2D.Double(p1, p2);
Line2D.Double rightRoof = new Line2D.Double(p1, p3);

This has the advantage that you only need to change the name at the part of the code where you draw the house:

g2.draw(leftRoof);
g2.draw(rightRoof);

You should definitely rename the variables p1, p2 and p3 as it is then easier to understand where those points are in your house drawing:

Point2D.Double topRoof = new Point2D.Double(x, y - halfMeasurement);
Point2D.Double leftBottomRoof = new Point2D.Double(x - halfMeasurement, y);
Point2D.Double rightBottomRoof = new Point2D.Double(x + halfMeasurement, y);

Refactoring

g2.draw(new Line2D.Double(topRoof, leftBottom));
g2.draw(new Line2D.Double(topRoof, rightBottom));

This is, in my opinion, the worse way of doing it, as it is not as easy to read when you instead have variables with meaningful names.

xCoord and yCoord

First of all, I would rename those to xCoordinate and yCoordinate but it is not as crucial as it is with the other variables and up to you. The declaration and first initialization are 100% fine. When changing xCoord and yCoord to be the coordinates for the door you access the getters for the x and y-coordinate of the point again instead of using the variables itself that already contain these coordinates. To simplify these two lines you can use += to simply increment the xCoord by measurement / 5 and the yCoord by measurement / 2:

xCoord += measurement / 5;
yCoord += halfMeasurement;

If you changed the variable measurement like I proposed to an int you don't even need to cast the result to an int value, as it automatically uses the correct arithmetic.

For the windows x-coordinate and y-coordinate you can use += again instead of using xCoord = xCoord + ...; to shorten the line and thereby making it easier to read and understand:

xCoord += (int) (measurement / 2.5);
yCoord += measurement / 10;

Here you will still need the int cast no matter what, as 2.5 is a double and thereby this calculation would return an int without the cast operator.

Comments

I would add some comments to your methods for better maintainability. In general it is recommended to create a JavaDoc comment for at least the class and every public member of the class.

Normal comments

If you need to add a comment to a line of code that is not that easy to understand, it is usually better to create that comment in the line that is above the line of code that needs a more detailed description as it is easier to read without having to scroll to the right and back.

Putting it all together

import java.awt.*;
import java.awt.geom.Line2D;
import java.awt.geom.Point2D;

/**
 * Draws a simple house using the Java Swing library.
 */
public class HouseCreator {

    private double x;
    private double y;
    private int measurement;

    /**
     * Constructor.
     *
     * @param x             The desired x-coordinate.
     * @param y             The desired y-coordinate.
     * @param measurement   The size of the house.
     */
    public HouseCreator(double x, double y, int measurement) {
        this.x = x;
        this.y = y;
        this.measurement = measurement;
    }

    /**
     * Draws a house that has a roof, a door and a window.
     *
     * @param graphics  A graphics instance that is used for drawing.
     */
    public void draw(Graphics2D graphics) {
        int halfMeasurement = measurement / 2;

        // Reference points of the roof.
        Point2D.Double topRoof = new Point2D.Double(x, y - halfMeasurement);
        Point2D.Double leftBottomRoof = new Point2D.Double(x - halfMeasurement, y);
        Point2D.Double rightBottomRoof = new Point2D.Double(x + halfMeasurement, y);

        // Roof lines.
        Line2D.Double leftRoof = new Line2D.Double(topRoof, leftBottomRoof);
        Line2D.Double rightRoof = new Line2D.Double(topRoof, rightBottomRoof);

        // Creates the house itself.
        int xCoord = (int) leftBottomRoof.getX();
        int yCoord = (int) leftBottomRoof.getY();

        Rectangle house = new Rectangle(xCoord, yCoord, measurement, measurement);

        // Creating the rectangle that represents the door of the house.
        xCoord += measurement / 5;
        yCoord += halfMeasurement;

        Rectangle door = new Rectangle(xCoord, yCoord,measurement / 4, halfMeasurement);

        // Creating the rectangle that represents the window of the house.
        xCoord += measurement / 2.5;
        yCoord += measurement / 10;

        Rectangle window = new Rectangle(xCoord, yCoord, measurement / 4, measurement / 4);

        // Draws all the components.
        graphics.draw(leftRoof);
        graphics.draw(rightRoof);
        graphics.draw(house);
        graphics.draw(door);
        graphics.draw(window);
    }
}

Answer 2

Thank you for sharing your code!

The most problematic part here is naming. Finding good names is the hardest part in programming. So always take your time to think about your identifier names.

You (obviously) already perceived the bad names if your identifiers and tried to work around this by adding comments. The better way is to change the identifier names.

    Point2D.Double roofTop = new Point2D.Double(x, y - (measurement / 2));   
    Point2D.Double roofBottomLeft = new Point2D.Double(x - (measurement / 2), y); 
    Point2D.Double roofBottomRigh = new Point2D.Double(x + (measurement / 2), y);

Other weaknesses in your code are Separation of Concerns / Single Responsibility Pattern an Same Level of Abstraction

Your method has roughly two parts:

1. Preparing the graphical elements
2. drawing them

You should separate them in individual methods:

public void draw(Graphics2D g2)
{
    prepareElements();
    drawElements(g2);
}

The problem here is that you create 5 objects but in Java a method returns only one value.

The solution could be to put the created objects in a collection of the type taken by the draw() method of Graphics2D:

public void draw(Graphics2D g2)
{
    Collection<Shape> houseElements = prepareElements();
    drawElements(g2, houseElements);
}

By the same pattern you could split up the prepareElements() method to create individual house elemnts:

private Collection<Shape> prepareElements(){
    Collection<Shape> houseElements = new HashSet<>();
    houseElements.addAll(createRoof());
    houseElements.add(createHouse());
    houseElements.add(createWindow());
    houseElements.add(createDoor());
    return houseElements;
}

This will lead to very small very specialized and easy to test methods.