Java : Assume object reference or make copy

Question

Consider the two classes:

public class Point {
    public int x;
    public int y;

    public Point(int xVal, int yVal) {
        x = xVal;
        y = yVal;
    }

    public Point(Point pt) {
        x = pt.x;
        y = pt.y;
    }
}

public class BoundingBox {
    public Point topLeft;
    public Point bottomRight;

    public BoundingBox(Point setTopLeft, Point setBottomRight) {
        topLeft = new Point(setTopLeft);
        bottomRight = new Point(setBottomRight);
    }
}

Should BoundingBox make a copy of the points passed into its constructor as shown, or just take a reference to them? If it assumes their reference values, is it guaranteed that those Point objects will exist as long as the BoundingBox exists?

Answer 1

two questions:

1. make a copy or not? It depends, if the point will be shared -- be used by other threads, then make a copy is better; If not shared, then just use reference may got tiny performance gain.

2. Is it guaranteed that those Point objects will exist as long as the BoundingBox exists? yes, it is guaranteed by the JVM. since the points are referenced by the BoundingBox, they will not be garbage collected.

Answer 2

You should always make defensive copies of stored mutable objects in your classes. You should always program defensively, assume that everyone that uses your class will break it.

This is demonstrated by the fact that although the class itself might be immutable, doesn't mean that objects in it are immutable too. You need to make defensive copies of mutable objects that you are using within your class.

Here's an example:

    public class MyClass {
           private Point foo;
           private Point bar;

           public MyClass(Point foo, Point bar) {
             this.foo = foo;
             this.bar = bar;
           }

           public Point foo() {
             return foo;
           }

           public Point bar() {
             return bar;
           }

           . . .

           //Seems harmless enough?
           //Lets destroy it
           Point foo = new Point(1 ,2);
           Point bar = new Point(3 ,4);
           MyClass mc = new MyClass(foo, bar);
           bar.x = 99; //<-- changes internal of mc!

This happens because the object MyClass only ever stored a pointer to the Point object that was passed in to it, which means that when the mutable object is changed - anything that points to that object is also changed. This can cause unintentional and unexepected results

To repair the above code, you make defensie copies of everything. It is important to note that copies should be made before any parameter checking occurs (e.g. validity check). You also need to make sure that your accessors are changed so that they too return copies of the internals of the class.

         //Fixed version!
            public MyClass(Point foo, Point bar) {
             this.foo = new Point(foo.getLocation());
             this.bar = new Point(bar.getLocation());
           }

           public Point foo() {
             return new Point(foo.getLocation());
           }

           public Point bar() {
             return new Point(bar.getLocation());
           }

           . . .

For the second part of your question - as long as BoundingBox exists, the objects contained within it should exist too. The JVM wont garbage collect those until all references to them no longer exist.

Answer 3

If you can make the Point object immutable, then you should be fine taking references just to the points. You can do this by declaring x, y fields as final as below:

public class Point {
    public final int x;
    public final int y;

    public Point(int xVal, int yVal) {
        x = xVal;
        y = yVal;
    }

    public Point(Point pt) {
        x = pt.x;
        y = pt.y;
    }
}

Holding references onto mutable objects would mean that a BoundingBox instance may just change it's dimensions if a topLeft, topRight reference were changed by any other class - leading to bugs that would be very hard to debug.

Also, Java reference counts objects, and they are not destroyed until all the references have been released. So, the points in your BoundingBox will be just fine as long as there are references to them.

Answer 4

If it assumes their reference values, is it guaranteed that those Point objects will exist as long as the BoundingBox exists?

Yes, the point objects will continue to exist until they're not used anymore. This is what the JVM garbage collection does for you.

Should BoundingBox make a copy of the points passed into its constructor as shown, or just take a reference to them?

You should make a copy of them. Otherwise, what happens when someone else comes along and does this:

Point topLeft = new Point(1, 2);
Point bottomRight = new Point(3, 4);
BoundingBox box = new BoundingBox(topLeft, bottomRight);

topLeft.x = 5; // Oops, this just changed box.topLeft.x

Generally it's good practice to avoid "gotchas" -- code that works in ways that's not expected. Even if you remember what your code's gotchas are now, the first time you forget one you're going to be really confused.

Answer 5

Neither way is right or wrong. It all depends on the specifics of your application. If your bounding box needs to rely on the points not to change from under it, then you have to make a copy in the constructor. But then you also need to make sure to only publish copies of the objects in any getter methods (e.g. if you end up having getTopLeft(), getBottomRight(), etc. More generally, this is a question of whether use composition vs. aggregation when designing your object model. Aggregation doesn't imply ownership. In other words, aggregated objects can exist outside the scope of their parent like in the case where you simply store the same point references that where passed to the constructor. With object composition, the life span of the child objects will be the same as that of the parent object. This is achieved by making copies of the points and then being careful about not passing out references of the stored points (only copies). This way, once the parent object goes away, so will the children.