Is it ok and safe to work with references passed to constructor - Java

Question

I've got the following code:

public class Triangle {

    private Point left;
    private Point right;
    private Point top;

    public Triangle(Point left, Point right, Point top) {
        this.left = left;
        this.right = right;
        this.top = top;
    }

    // more program logic...
}

I was wondering if it's okay and safe to construct an object like that, because I fear that some of the three variables of type Point could be modified from the outside(break encapsulation). For example:

public static void main(String[] args) {

    Point left = new Point(0.0, 1.0);
    Point right = new Point(2.4, 3.2);
    Point top = new Point(5.8, 2.0);

    Triangle t = new Triangle(left, right, top);

    top.setX(10.2);
    top.setY(23.4); 
}

This will undoubtedly manipulate the same "top" object that's being referenced in the Triangle variable. So is the fix doing the following inside the Triangle constructor:

public Triangle(Point left, Point right, Point top) {
    this.left = new Point(left);
    this.right = new Point(right);
    this.top = new Point(top);
}

(keep in mind that I have a copy constructor in the Point class, so the three statements above are valid)

Answer 1

You could clone the original points in the constructor and keep the clones hidden from the outside world. If Point already implements Cloneable or if you can implement it yourself, use it this way:

public Triangle(Point left, Point right, Point top) {
    this.left = left.clone();
    this.right = right.clone();
    this.top = top.clone();
}

If Point doesn't implement Cloneable and you have no access to its source code, just clone points manually:

public Triangle(Point left, Point right, Point top) {
    this.left = new Point(left.getX(), left.getY());
    this.right = new Point(right.getX(), right.getY());
    this.top = new Point(top.getX(), top.getY());
}

Answer 2

Its a very good question to ask.

Its not always a bad thing have mutable state. For example, you could send this Triangle object instance to a display program, and if you changed the point co-ordinates, it could animate the triangle on screen.

Its upto the use case and the problem that it is supposed to solve.

If you made up your mind that for your use case, the entire object graph (this object, my children and children's children, etc.) should be immutable once created, there are ways to ensure that.

For making an immutable object really immutable, there is some good guidance here: http://www.javapractices.com/topic/TopicAction.do?Id=29

If the object has collections as properties, you could use Immutable or Unmodifiable collections as required: Java Immutable Collections

Finally, if you did allow mutable state, and wanted to keep track, you could register with the child object as an observer. When should we use Observer and Observable

Again, the principle is to keep as little of mutable state as possible. In other words, always keep objects Immutable as far as possible. Immutable collections are easy to come by and easy to use. The problem is with our custom classes.

If you made your custom object/model object completely immutable, you may have situations where you would be copying entire object graphs to change a single property.

So, be conservative. Especially with collections, it is easy to do.

With Object oriented programming paradigm it is difficult to avoid mutability. Functional programming is supposed to be lot more aligned with immutability. You may want to check out Haskell or Scala just to get a taste.

https://softwareengineering.stackexchange.com/questions/232711/complete-immutability-and-object-oriented-programming

Answer 3

If you have a copy constructor, then you're safe with using that. Otherwise, you can use clone if point implements cloneable.