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

Check if one shape is in another shape

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

Problem

I have a code to check if one shape (circle or square) envelops another. I have used this code. It gives true if the this object envelops the passed shape. Can anyone suggest me an improvement or better alternate method?

public boolean envelops(Shape s) { 
boolean flag = true;
double distance = 0.0;
if(s instanceof Square){ //check if the shape is a Square
 Point bottomLeft= new Point(((Square)s).getTopLeft().getX(),((Square)s).getTopLeft().getY()-((Square)s).getSideLength()); //calculating the cordinates of the square 
 Point bottomRight= new Point(((Square)s).getTopLeft().getX()+((Square)s).getSideLength(),((Square)s).getTopLeft().getY()-((Square)s).getSideLength());   
 Point topRight= new Point(((Square)s).getTopLeft().getX()+  ((Square)s).getSideLength(),((Square)s).getTopLeft().getY());

 ArrayList points = new ArrayList(); //storing points in a ArrayList of points
 points.add(((Square)s).getTopLeft());
 points.add(bottomLeft);
 points.add(topRight);
 points.add(bottomRight);
    for(int i=0; iShape.TOLERANCE) flag= false;      //if the distance is greater than the radius then flag becomes false, ie., the cordinate is outside the circle                                          

       }                    
 } 

if(s instanceof Circle){
 distance=this.center.distance(((Circle)s).getCenter());  //finding distance between the centers of the 2 circles
 distance= distance +((Circle)s).getRadius();
 if(distance- this.getRadius()> Shape.TOLERANCE || distance==this.getRadius()){ //The method will return false if both circles have 0 radius
    flag=false;                                           //make flag false if the distance is more than the radius of the object Circle
 }
}

return flag;
}

Solution

Your indentation is quite messy. Indentation is usually 4 spaces in Java, so use 4 spaces.

Including some other minor readability improvements (such as each line is a maximum of 80 characters), your code will look like:

public boolean envelops(Shape s) { 
    boolean flag = true;
    double distance = 0.0;
    if (s instanceof Square) { // check if the shape is a Square 
        // calculating the coordinates of the square 
        Point bottomLeft = new Point(((Square) s).getTopLeft().getX(), 
                ((Square) s).getTopLeft().getY() - ((Square) s).getSideLength());
        Point bottomRight = new Point(((Square) s).getTopLeft().getX() + 
                ((Square) s).getSideLength(), 
                ((Square) s).getTopLeft().getY() - ((Square) s).getSideLength());   
        Point topRight = new Point(((Square) s).getTopLeft().getX() + 
                ((Square) s).getSideLength(), ((Square) s).getTopLeft().getY());

        // storing points in a ArrayList of points
        ArrayList points = new ArrayList();
        points.add(((Square) s).getTopLeft());
        points.add(bottomLeft);
        points.add(topRight);
        points.add(bottomRight);
        for (int i = 0; i  Shape.TOLERANCE) flag = false;
        }
    } 

    if (s instanceof Circle) {
        // finding distance between the centers of the 2 circles
        distance = this.center.distance(((Circle) s).getCenter());
        distance = distance + ((Circle) s).getRadius();
        // The method will return false if both circles have 0 radius
        if (distance - this.getRadius() > Shape.TOLERANCE
                 || distance == this.getRadius()) {
            // make flag false if the distance is more than
            // the radius of the object Circle
            flag = false;
        }
    }

    return flag;
}


Some other points:

You don't need to cast to Square or Circle every time. Instead, create a new Square or Circle:

Square square = (Square) s;


and:

Circle circle = (Circle) s;


You don't need to keep a flag that you will eventually return with. Just return directly.

You don't need an ArrayList: just use an array.
Final Code

public boolean envelops(Shape s) {
    double distance = 0.0;
    if (s instanceof Square) { // check if the shape is a Square
        Square square = (Square) s;
        // calculating the coordinates of the square 
        Point bottomLeft = new Point(square).getTopLeft().getX(), 
                square.getTopLeft().getY() - square.getSideLength());
        Point bottomRight = new Point(square.getTopLeft().getX() + 
                square.getSideLength(), 
                square.getTopLeft().getY() - square.getSideLength());   
        Point topRight = new Point(square.getTopLeft().getX() + 
                square.getSideLength(), square.getTopLeft().getY());

        // storing points in a ArrayList of points
        Point[] points = { square.getTopLeft(), bottomLeft, topRight, bottomRight };
        for (int i = 0; i  Shape.TOLERANCE) return false;
        }
    } 

    if (s instanceof Circle) {
        Circle circle = (Circle) s;
        // finding distance between the centers of the 2 circles
        distance = this.center.distance(circle.getCenter());
        distance = distance + circle.getRadius();
        // The method will return false if both circles have 0 radius
        if (distance - this.getRadius() > Shape.TOLERANCE
                 || distance == this.getRadius()) {
            // make flag false if the distance is more than
            // the radius of the object Circle
            return false;
        }
    }

    return true;
}

Code Snippets

public boolean envelops(Shape s) { 
    boolean flag = true;
    double distance = 0.0;
    if (s instanceof Square) { // check if the shape is a Square 
        // calculating the coordinates of the square 
        Point bottomLeft = new Point(((Square) s).getTopLeft().getX(), 
                ((Square) s).getTopLeft().getY() - ((Square) s).getSideLength());
        Point bottomRight = new Point(((Square) s).getTopLeft().getX() + 
                ((Square) s).getSideLength(), 
                ((Square) s).getTopLeft().getY() - ((Square) s).getSideLength());   
        Point topRight = new Point(((Square) s).getTopLeft().getX() + 
                ((Square) s).getSideLength(), ((Square) s).getTopLeft().getY());

        // storing points in a ArrayList of points
        ArrayList<Point> points = new ArrayList<Point>();
        points.add(((Square) s).getTopLeft());
        points.add(bottomLeft);
        points.add(topRight);
        points.add(bottomRight);
        for (int i = 0; i < points.size(); i++) {
            //finding ditance of each cordinate from the center of the circle
            distance = this.center.distance(points.get(i));
            // if the distance is greater than the radius then flag becomes false,
            // ie., the cordinate is outside the circle
            if (distance - this.radius > Shape.TOLERANCE) flag = false;
        }
    } 

    if (s instanceof Circle) {
        // finding distance between the centers of the 2 circles
        distance = this.center.distance(((Circle) s).getCenter());
        distance = distance + ((Circle) s).getRadius();
        // The method will return false if both circles have 0 radius
        if (distance - this.getRadius() > Shape.TOLERANCE
                 || distance == this.getRadius()) {
            // make flag false if the distance is more than
            // the radius of the object Circle
            flag = false;
        }
    }

    return flag;
}
Square square = (Square) s;
Circle circle = (Circle) s;
public boolean envelops(Shape s) {
    double distance = 0.0;
    if (s instanceof Square) { // check if the shape is a Square
        Square square = (Square) s;
        // calculating the coordinates of the square 
        Point bottomLeft = new Point(square).getTopLeft().getX(), 
                square.getTopLeft().getY() - square.getSideLength());
        Point bottomRight = new Point(square.getTopLeft().getX() + 
                square.getSideLength(), 
                square.getTopLeft().getY() - square.getSideLength());   
        Point topRight = new Point(square.getTopLeft().getX() + 
                square.getSideLength(), square.getTopLeft().getY());

        // storing points in a ArrayList of points
        Point[] points = { square.getTopLeft(), bottomLeft, topRight, bottomRight };
        for (int i = 0; i < points.length; i++) {
            //finding distance of each coordinate from the center of the circle
            distance = this.center.distance(points[i]);
            // if the distance is greater than the radius then flag becomes false,
            // ie., the cordinate is outside the circle
            if (distance - this.radius > Shape.TOLERANCE) return false;
        }
    } 

    if (s instanceof Circle) {
        Circle circle = (Circle) s;
        // finding distance between the centers of the 2 circles
        distance = this.center.distance(circle.getCenter());
        distance = distance + circle.getRadius();
        // The method will return false if both circles have 0 radius
        if (distance - this.getRadius() > Shape.TOLERANCE
                 || distance == this.getRadius()) {
            // make flag false if the distance is more than
            // the radius of the object Circle
            return false;
        }
    }

    return true;
}

Context

StackExchange Code Review Q#109389, answer score: 6

Revisions (0)

No revisions yet.