Generating 50 Random Shapes

Question

I am a beginner Java programmer. I have just finished up an assignment and would appreciate some advice and/or constructive criticism on my program. For context, I have the assignment details, followed by the full code below:

The Shape interface is as follows:

public interface Shape {
        public double area();
        public double perimeter();
        public double getXPos();
        public double getYPos();
        public void move(double xLoc, double yLoc);
        public String toString();
}

An AbstractShape has the following attributes:

• xPos
• yPos

An AbstractShape has the following behaviour:

• move(x,y)
• getXPos
• getYPos
• toString

A Circle adds the following attributes:

• radius

A Circle adds the following behaviour:

• area
• perimeter
• equals(Circle)
• clone
• updated toString

A Rectangle adds the following attributes:

• height
• width

A Rectangle adds the following behaviour:

• area
• perimeter
• equals(Rectangle)
• clone
• updated toString

Also include constructors in your classes.

Write a test harness which generates 50 random Shapes, stores them in an array, then prints them out.

Main Class: RandomShapes

public class RandomShapes {

    public static void main(String[] args) {
        // Creating the random variables
        double randX, randY, randRadius, randWidth, randHeight;

        // Array to store AbstractShapes
        AbstractShape[] shapes = new AbstractShape[50];

        for (int i = 0; i < shapes.length; i++) {
            // Random number between 1-2 to decide rectangle, or circle
            int circleOrRect = (int) (Math.random() * 2 + 1);

            // Creating 6 random numbers
            randX = (int) ((Math.random() * 100) + 1);
            randY = (int) ((Math.random() * 100) + 1);
            randRadius = (int) ((Math.random() * 100) + 1);
            randWidth = (int) ((Math.random() * 100) + 1);
            randHeight = (int) ((Math.random() * 100) + 1);

            // If random number is 1, create/add a circle to array, else, add rectangle
            if (circleOrRect == 1) {
                // Create new circle with random (x, y)
                shapes[i] = new Circle(randX, randY);
                // Cast AbstractShape to Circle and set random radius
                ((Circle) shapes[i]).setRadius(randRadius);
                // Print out the object's info
                System.out.println("\n#" + (i+1) + shapes[i].toString());
            } else {
                // Create new rectangle with random (x, y)
                shapes[i] = new Rectangle(randX, randY);
                // Cast AbstractShape to Rectangle and set random height/width
                ((Rectangle) shapes[i]).setHeight(randHeight);
                ((Rectangle) shapes[i]).setWidth(randWidth);
                // Print out the object's info
                System.out.println("\n#" + (i+1) + shapes[i].toString());
            }
        }
    }
}

Interface: Shape

public interface Shape {

    // All the abstract methods
    public double area();

    public double perimeter();

    public double getXPos();

    public double getYPos();

    public void move(double xLoc, double yLoc);

    public String toString();
}

AbstractClass: AbstractShapes

abstract public class AbstractShape implements Shape {

    // X and Y positions
    protected double xpos;
    protected double ypos;

    // Constructor for an AbstractShape
    public AbstractShape(double x, double y) {
        xpos = x;
        ypos = y;
    }

    // To move a shape
    public void move(double xLoc, double yLoc) {
        xpos = xLoc;
        ypos = yLoc;
    }

    // Get X position of a shape
    public double getXPos() {
        return xpos;
    }

    // Get Y position of a shape
    public double getYPos() {
        return ypos;
    }

    //Generic toString method
    public String toString() {
        String message = "xPos: " + xpos
                + "\nyPos: " + ypos;
        return message;
    }
}

Sub-Class: Circle

public class Circle extends AbstractShape {

    // Radius variable
    private double radius;

    // Constructor is the same as AbstractShape
    public Circle(double xpos, double ypos) {
        super(xpos, ypos);
    }

    // Set radius value
    public void setRadius(double r) {
        radius = r;
    }

    // Get radius value
    public double getRadius() {
        return radius;
    }

    // Calculate area
    public double area() {
        double area = Math.PI * Math.pow(radius, 2);
        return area;
    }

    // Calculate perimeter (circumference)
    public double perimeter() {
        double perimeter = 2 * Math.PI * radius;
        return perimeter;
    }

    // Clone the object
    public Circle clone() {
        Circle theClone;
        theClone = new Circle(this.xpos, this.ypos);
        theClone.setRadius(this.radius);
        return theClone;
    }

    // Check to see if 2 objects are the same
    public boolean equals(Circle c) {
        return c.radius == this.radius
                && c.getXPos() == this.xpos
                && c.getYPos() == this.ypos;
    }

    // Specific toString() method
    public String toString() {
        String message = "\n[CIRCLE]"
                + "\nPosition: (" + xpos + ", " + ypos
                + ")\nRadius: " + radius
                + "\nArea: " + this.area()
                + "\nCircumference: " + this.perimeter();
        return message;
    }
}

Sub-Class: Rectangle

public class Rectangle extends AbstractShape {

    // Height and width variables
    private double height;
    private double width;

    // Constructor for rectangle (same as super)
    public Rectangle(double xpos, double ypos) {
        super(xpos, ypos);
    }

    // Set height value
    public void setHeight(double h) {
        height = h;
    }

    // Get the height value
    public double getHeight() {
        return height;
    }

    // Set the width value
    public void setWidth(double w) {
        width = w;
    }

    // Get the width value
    public double getWidth(double w) {
        return width;
    }

    // Calculate the area
    public double area() {
        double area = height * width;
        return area;
    }

    // Calculate the perimeter
    public double perimeter() {
        double perimeter = (2 * width) + (2 * height);
        return perimeter;
    }

    // Clone the object
    public Rectangle clone() {
        Rectangle theClone;
        theClone = new Rectangle(this.xpos, this.ypos);
        theClone.setHeight(this.height);
        theClone.setWidth(this.width);
        return theClone;
    }

    // Check to see if 2 objects are the same
    public boolean equals(Rectangle r) {
        return r.height == this.height
                && r.width == this.width
                && r.getXPos() == this.xpos
                && r.getYPos() == this.ypos;
    }

    // Specific toString() method
    public String toString() {
        String message = "\n[RECTANGLE]"
                + "\nPosition: (" + xpos + ", " + ypos
                + ")\nHeight: " + height
                + "\nWidth: " + width
                + "\nArea: " + this.area()
                + "\nPerimeter: " + this.perimeter();
        return message;
    }
}

Answer 1

I'll just review RandomShapes. I figure there's not much to go wrong when you are given a design that clear:

Comments

You're overcommenting. This is one of the things that annoys me most when reading code that's written for assignments. The comments have been written for the sake of there being comments. That's absolutely not the point of comments. Comments that merely restate what's already written in code are useless, if not dangerous.

I strongly advise you to not use comments that way.

Use of random

You're repeatedly using Math.random(). Since you're using it to get a random number in the range of [1..100] you may want to check into using the Random object from the java library.

Consider the following code:

Random rng = new Random();
randX = rng.nextInt(100) + 1;
randY = rng.nextInt(100) + 1;
// ...

This removes all the casts from your random parameter creation.

Variable Scopes and Types

The random variables are declared outside the for-loop, but not used after. It's generally best practice to declare variables as close as possible to their usage and in the smallest scope possible.

This allows you to keep the enclosing scope clean and eases programming, especially in longer blocks.

Additionally using an int to store a binary choice (between circle and rectangle) is not a good idea. Since you only have Circles and Rectangles, you should consider storing circleOrRect in a boolean. You can obtain a random boolean from a Random object by invoking nextBoolean()

One additional thing. The code directly stores the shapes it creates in the shapes array. This makes it needlessly difficult to set the interesting values of radius or width and height respectively.

One option to fix this would be to introduce constructors to the subclasses that take all the relevant arguments as parameters. Consider:

if (circle) {
    shapes[i] = new Circle(randX, randY, randRadius);
} else {
    shapes[i] = new Rectangle(randX, randY, randWidth, randHeight);
}

Code duplication and placement

One last thing: When two branches of code do the same thing, one would strive to merge them. When two branches of code need different things, one would strive to only generate the resources necessary for the code when it's known what will be necessary.

I'd rewrite the for-loop as follows:

for (int i = 0; i < shapes.length; i++) {
    boolean circle = rng.nextBoolean();
    int randX = rng.nextInt(100) + 1;
    int randY = rng.nextInt(100) + 1;
    if (circle) {
        int randRadius = rng.nextInt(100) + 1;
        shapes[i] = new Circle(randX, randY, randRadius);
    } else {
        int randWidth = rng.nextInt(100) + 1;
        int randHeight = rng.nextInt(100) + 1;
        shapes[i] = new Rectangle(randX, randY, randWidth, randHeight);
    }
    System.out.println("\n#" + (i + 1) + shapes[i].toString());
}

This minimizes the effort expended and still accomplishes the same result

Answer 2

Reusability and code safety

Since a lot has already been covered, I will focus on defensive programming to make your code safer, especially safer to reuse.

Let's consider Circle and its constructor and method:

public Circle(double xpos, double ypos)

public void setRadius(double r)

You declare this class to be public. That means anyone can create a Circle instance. You never know what kind of things people will try to stuff into your constructor, but you want your Circle to be safe and correct.

What happens if someone passes a negative radius? You will and up with an illegal circle. That is something you want to prevent.

Consider this addition:

    // Set radius value. Must be positive!
    public void setRadius(double r) {

        if (r < 0.0) { 
            throw new IllegalArgumentException("Radius cannot be negative!"); 
        }

        radius = r;
    }

This makes sure your classes stay valid, and are better reusable by others.

Answer 3

Suggestions

You have written pretty good code already, and your describe your methods with comments just before them. You could make those comments into property JavaDoc comments, see the Oracle JavaDoc tutorial here.