What is the correct encapsulation in java object types

Question

What is the correct encapsulation from below 2 classes in java?.I saw both these approches in many codes(mostly 1st approche). But seems like 2nd approach is the correct one.

import java.util.Date;

public class SomeClass
{
    private Date date;

    public Date getDate()
    {
        return date;
    }

    public void setDate(Date date)
    {
        this.date = date;
    }   
}

or

import java.util.Date;

public class SomeClass
{
    private Date date;

    public Date getDate()
    {
        return (Date) date.clone();
    }

    public void setDate(Date date)
    {
        this.date = (Date) date.clone();
    }

}

Answer 1

Two basic feature of encapsulation are:

• Keep instance variables protected (with an access modifier, often private).
• Make public accessor methods, and force calling code to use those methods rather than directly accessing the instance variable

It is always a good practice to create defensive copy of the mutable object any time it is passed into Constructors and set methods or out of get methods in the class. If this is not done, then it is simple for the caller to break encapsulation, by changing the state of an object which is simultaneously visible to both the class and its caller. Also, Don't use the clone method to make defensive copy of a parameter(mutable object) whose type is subclassable by untrusted parties as this may lead to intentional or unintentional change in the internal state of the instance variable.
Going by all these rules, none of your approach is correct.

Hence , the correct way to follow encapsulation in your code is:

import java.util.Date;

public class SomeClass
{
    private Date date;

    public Date getDate()
    {
        return new Date(date.getTime());
    }

    public void setDate(Date date)
    {
        this.date = new Date(date.getTime());
    }

}

Answer 2

It depends whether the type of the field you get/set is immutable or not, i.e. if they can be modified after their construction.

The point behind the whole Getter/Setter paradigm is that the private/protected fields of an instance cannot be arbitrarily modified (or gained access to) by any external Class.

So, in your first example, a Class could acquire a reference to the private Date-field and then (since Date is not immutable) use Date's setTime(long) method to modify the Date, effectively bypassing the Setter method of SomeClass (and any side-effects it might have, e.g. performing validation, updating a GUI element etc).
In your second example this can't be done, since the external Class would only acquire a clone of the actual Date, so any modifications done after that would have no effect on the original private Date-field of SomeClass.

Bottom line:
It all depends on the type of your private/protected fields and what you are trying to achieve/prevent.

---

Points to keep in mind:

• clone() will not always return a deep-clone of an Object (especially for complex Object, whose fields reference other mutable Objects etc). So it must be used with care (and awareness of its inner workings).

• Primitive data-types and Strings are immutable, so there is no need for clone()ing when getting/setting fields of these types.

Answer 3

Not a good idea to clone as -

1. Generally clone method of an object, creates a new instance of the same class and copies all the fields to the new instance and returns it = shallow copy. Object class provides a clone method and provides support for the shallow copy. It returns ‘Object’ as type and you need to explicitly cast back to your original object.

2. When you implement deep copy be careful as you might fall for cyclic dependencies.

3. Clone is not for instantiation and initialization. It should not be taken as creating a new object. Because the constructor of the cloned objects may never get invoked in the process.

4.One more disadvantage (and many others.. :)), clone prevents the use of final fields.

5 . In a Singleton pattern, if the superclass implements a public clone() method, to prevent your subclass from using this class’s clone() method to obtain a copy overwrite it and throw a CloneNotSupportedException

So, Approach 1 is better than Approach 2

Answer 4

The second tries to enforce Defensive Copying. Consider a class Period, that stores two dates and the first must be before the second:

public class Period {
    private Date first, second;

    public Period(Date first, Date second) {
        if(first.compareTo(second) > 0) 
            throw new IllegalArgumentException("first > second");

        this.first = first;
        this.second = second;
    }

    public Date getFirst() {
        return first;
    }

    public Date getSecond() {
        return second;
    }
}

On the first glance, this sounds unhackable, but take a look:

Date d1 = new Date(), d2 = new Date();
Period p = new Period(d1, d2) // no exception

d1.setYear(98); // broken period precondition

To solve this - defensive copying comes in, that is, that the internal instance can not be changed by the original parameter. Although your second approach will work, it is still hackable, as a subclass can override clone() and save all new instances created...

The right way would be:

this.first = new Date(first.getTime());

And the return statements:

return new Date(first.getTime()); // here you may use clone....

This way, there's no way for a subclass to get a hand on the internals.

Answer 5

This is a matter of security/encapsulation preference, the most basic encapsulation is the 1st approach you posted, the second however is a more advanced way of encapsulation. This protects also the objects being passed on to the class by cloning it.

Consider the following:

public class SomeData {
 private final Point value;
  public SomeData (final Point value) {
    this.value = value;
  }
  public Point getValue( ) {
    return value;
  }
}

now the above snippet looks immutable (similar to your example) However there is a gaping hole in this. check out the snippet below.

  public final static void main(final String[] args) {
    Point position = new Point(25, 3);
    SomeData data = new SomeData(position);
    position.x = 22;
    System.out.println(data.getValue( ));
  }

since we only pass the reference of the position variable we are still able to alter its state. Cloning it would help protect the variable position:

if we change the declaration from this:

public SomeData (final Point value) {
        this.value = value;
      }

to (similar to your cloning)

 public SomeBetterData(final Point value) {
    this.value = new Point(value);
  }

when we invoke main method again:

Point position = new Point(25, 3);
        SomeData data = new SomeData(position);
        position.x = 22;

the data object will remain unchanged regardless of what we do to position. I hope you get the idea why there is a cloning there.

Answer 6

The first one ! The second one will create a new object with clone() for each getDate() call, it could be embarassing, depending on your application. (ie. if you want to call a Date method with aSomeDate.getDate().aMethod())

A little sample to understand my poor english ;)

public class Main {
  public static void main(String[] args) {
    SomeDate sd = new SomeDate(new Date(1991, 3, 3));
    System.out.println(sd);
    sd.getDate().setYear(2012);
    System.out.println(sd);
  }
}

Answer 7

Not for this specific example but for a general approach:

Its always good to provide the user a way to cast the returned object into any other compatible class he wants.

So the first method looks good, because if the object you are returning have been extended by some other class then it will be easy to cast it into that type, rather than providing a fixed type object.

So this way you provide a more general object or I can say abstraction and encouraging polymorphism.

Answer 8

This depends on the code that uses SomeClass. If you plan to include this class in a library that is used by a third party or your code interacts/runs code you don't control, you absolutely need encapsulation of the latter form.

Even when you're not in such a difficult position, it's worth shielding from the design error that is the mutable Date class and return a new Date instance. That's the reason all IDEs I know flag this as warning.

Though that usually leads to something like this for me:

public class MyClass {
  private Date myDate;

  public Date getDate() { 
    return myDate != null ? new Date(myDate.getTime()) : null; 
  }
  public void setDate(Date date) { 
    myDate = (date != null ? new Date(date.getTime()) : null); 
  }
}

So IMHO, I'd use the second variant or switch to Jodas DateTime.