Object oriented design - Shapes

Question

So, after many years of OOP, I got a pretty simple homework assignment from one of my university courses to implement a simple object-oriented structure.

The requested design:

• Implement an objected oriented solution for creating the following shapes:

• Ellipse, Circle, Square, Rectangle, Triangle, Parallelogram.

• Each shape created must have the following parameters : unique id, color.

And following functions: color change, move, area, circumference, is inside, copy.

No validity tests are needed (not in the classes and not from user input).

My design:

Overall a pretty simple approach, shape_class / non-circular are abstract, and rectangle/square are combined into one class since they contain exactly the same parameters and no validity tests are needed (no reason to split them into two).

Shape class - implements a static id (unique id), and an init function dealing with the color name.

public abstract class shape_class {

    static int STATIC_ID;
    int id;
    String color_name;

    public shape_class(String color_name_input) {
        this.id = STATIC_ID;
        shape_class.STATIC_ID+=1;
        if (Arrays.asList(toycad_globals.ALLOWED_COLORS).contains(color_name_input))
        {
            this.color_name = color_name_input;
        }
    }

    public void change_color(String color_name_input) {
        if (Arrays.asList(toycad_globals.ALLOWED_COLORS).contains(color_name_input)) {
            this.color_name = color_name_input;
        }
    }

    public abstract shape_class return_copy();
    public abstract void move(double x, double y);
    public abstract double area();
    public abstract double circumference();
    public abstract boolean is_inside(double x, double y);
}

** Non-circular** - Receives an array of points (Which define the object) and implements almost all required functions.

public abstract class non_circullar extends shape_class {
    List<line> line_list = new ArrayList<line>();
    List<point> point_list = new ArrayList<point>();

    non_circullar(String color_name, point...input_point_list) {
        super(color_name);
        this.point_list = Arrays.asList(input_point_list);
        for (int current_index =0; current_index< (input_point_list.length); current_index++) {
            point current_first_point = input_point_list[current_index];
            point current_second_point = input_point_list[(current_index+1)%input_point_list.length];
            this.line_list.add(new line(current_first_point, current_second_point));
        }
    }

    public point[] get_point_list_copy() {
        int index = 0;
        point [] new_array = new point[this.point_list.size()];
        for (point current_point:this.point_list) {
            new_array[index] = current_point.return_copy();
            index+=1;
        }
        return new_array;
    }

    public double circumference() {
        double sum = 0;
        for (line current_line :this.line_list) {
            sum += current_line.get_length();
        }
        return sum;
    }

    public void move(double x, double y) {
        for (point current_point :this.point_list) {
            current_point.move(x, y);
        }
    }

    public boolean is_inside(double x, double y) {
        int i;
        int j;
        boolean result = false;
        for (i = 0, j = this.point_list.size() - 1; i < this.point_list.size(); j = i++) {
            if ((this.point_list.get(i).y > y) != (this.point_list.get(j).y > y) &&
                (x < (this.point_list.get(j).x - this.point_list.get(i).x) * (y - this.point_list.get(i).y) / 
                        (this.point_list.get(j).y-this.point_list.get(i).y) + this.point_list.get(i).x)) 
           {
              result = !result;
           }
        }
        return result;
    }

    int get_top_left_line_index() {
        int top_left_line_index = 0;
        int index = 0;
        point best_point = this.line_list.get(0).get_average_point();
        point current_point;
        for (line current_line :this.line_list) {
            current_point = current_line.get_average_point();

            if (current_point.x < best_point.x) {
                best_point = current_point;
                top_left_line_index = index;
            } else if (current_point.x == best_point.x && current_point.y > best_point.y) {
                best_point = current_point;
                top_left_line_index = index;
            }
            index +=1;
        }
        return top_left_line_index;
    }
}

The problem:

For this assignment 40 points were reduced for design issues:

1) Circle is an ellipse and thus needs to inherit from it (Even though they share no parameters).

2) Rectangle / Square are two different entities even though in this implementation they are exactly the same (no validity tests).

I would be happy to get some inputs from the community regarding this design, are the design issues 'legit' or not, and what could have been done better?

Edit 1:

An ellipse is expressed by : two points and d (For a point to be on the ellipse the distance between it and the two points must be equal to d).

A circle is expressed by : center and radius.

I find it very hard to understand how they can share common params.

Answer 1

I suggest you follow this scheme:

You need to categorize shapes by the number of the edges first and then by the common characteristics. Then you have to recognize the following facts:

• circle is just a special type of ellipse
• square is just a special type of rectangle
• both rectangle and parallelogram have 4 edges
• unlikeparallelogram, rectangle have all the angles of 90°.

This is a simplified scheme according to your needs:

Ellipse, Circle, Square, Rectangle, Triangle, Parallelogram

Edit: Note that there exists the following hierarchy as well. Both rectangle and parallelogram have the opposite edges of the same length. Finally, it depends on the preferred interpretation and on what suits your situation better (thanks to @Federico klez Culloca):

Quadrilateral <- Parallelogram <- Rectangle <- Square

Make it scalable: In case of more complex shapes of elementary geometry included, I'd put probably place polygon below shape and then differentiate the descendants by the convexity and non-convexity first.

Answer 2

The design you have used is not idea (IMHO).

First, rename non-circular into Polygon (Also, us uppercase for the first letter).

Based on the implementation, a Circle is a specific Ellipse so I would have used inheritance here

Shape < -- Circular < -- Ellipse < -- Circle
      < -- Polygon < -- Triangle      < -- Equilateral
                                      < -- ... //don't know the english names of those triangles 
                   < -- Quadrilateral < -- Square
                                      < -- Rectangle
                                      < -- ...
                   < -- Hexagon
                   < -- ...

Each subclass of Polygon are abstract, those are used for the validation of the number of corners.

In general, I would have linked Square and Rectangle based on the geometry rule (same width and heigth) ( Square extends Rectangle) but based on your implementation using Point and Line, this is not required.
But using two classes would still allows some validation in the future ( every Line for a Square need to have the same length, ...).

This shows that a design depends mostly on the requirement, not only on the subject.

About Ellipse and Circle. An Ellipse is form of two points, if those point are the same, this is a Circle, this can be a link ;)