Tutorial :Is clone() really ever used? What about defensive copying in getters/setters?



Question:

Do people practically ever use defensive getters/setters? To me, 99% of the time you intend for the object you set in another object to be a copy of the same object reference, and you intend for changes you make to it to also be made in the object it was set in. If you setDate ( Date dt ) and modify dt later, who cares? Unless I want some basic immutable data bean that just has primitives and maybe something simple like a Date, I never use it.

As far as clone, there are issues as to how deep or shallow the copy is, so it seems kind of "dangerous" to know what is going to come out when you clone an Object. I think I have only used clone() once or twice, and that was to copy the current state of the object because another thread (ie another HTTP request accessing the same object in Session) could be modifying it.

Edit - A comment I made below is more the question:

But then again, you DID change the Date, so it's kind of your own fault, hence whole discussion of term "defensive". If it is all application code under your own control among a small to medium group of developers, will just documenting your classes suffice as an alternative to making object copies? Or is this not necessary, since you should always assume something ISN'T copied when calling a setter/getter?


Solution:1

From Josh Bloch's Effective Java:

You must program defensively with the assumption that clients of your class will do their best to destroy its invariants. This may actually be true if someone tries to break the security of your system, but more likely your class will have to cope with unexpected behavior resulting from honest mistakes on the part of the programmer using your API. Either way, it is worth taking the time to write classes that are robust in the face of ill-behaved clients.

Item 24: Make defensive copies when needed


Solution:2

This is a non-trivial question. Basically, you have to think about any internal state of a class that you give to any other class via getter or by calling another class' setter. For example, if you do this:

Date now = new Date();  someObject.setDate(now);  // another use of "now" that expects its value to not have changed  

then you potentially have two problems:

  1. someObject could potentially change the value of "now", meaning the method above when it later uses that variable could have a different value than it expected, and
  2. if after passing "now" to someObject you change its value, and if someObject did not make a defensive copy, then you've changed the internal state of someObject.

You should either protected against both cases, or you should document your expectation of what is allowed or disallowed, depending on who the client of the code is. Another case is when a class has a Map and you provide a getter for the Map itself. If the Map is part of the internal state of the object and the object expects to fully manage the contents of the Map, then you should never let the Map out. If you must provide a getter for a map, then return Collections.unmodifiableMap(myMap) instead of myMap. Here you probably do not want to make a clone or defensive copy due to the potential cost. By returning your Map wrapped so that it cannot be modified, you are protecting your internal state from being modified by another class.

For many reasons, clone() is often not the right solution. Some better solutions are:

  1. For getters:
    1. Instead of returning a Map, return only Iterators to either the keySet or to the Map.Entry or to whatever allows client code to do what it needs to do. In other words, return something that is essentially a read-only view of your internal state, or
    2. Return your mutable state object wrapped in an immutable wrapper similar to Collections.unmodifiableMap()
    3. Rather than returning a Map, provide a get method that takes a key and returns the corresponding value from the map. If all clients will do with the Map is get values out of it, then don't give clients the Map itself; instead, provide a getter that wraps the Map's get() method.
  2. For constructors:
    1. Use copy constructors in your object constructors to make a copy of anything passed in that is mutable.
    2. Design to take immutable quantities as constructor arguments when you can, rather than mutable quantities. Sometimes it makes sense to take the long returned by new Date().getTime(), for example, rather than a Date object.
    3. Make as much of your state final as possible, but remember that a final object can still be mutable and a final array can still be modified.

In all cases, if there is a question about who "owns" mutable state, document it on the getters or setters or constructors. Document it somewhere.

Here's a trivial example of bad code:

import java.util.Date;    public class Test {    public static void main(String[] args) {      Date now = new Date();      Thread t1 = new Thread(new MyRunnable(now, 500));      t1.start();      try { Thread.sleep(250); } catch (InterruptedException e) { }      now.setTime(new Date().getTime());   // BAD!  Mutating our Date!      Thread t2 = new Thread(new MyRunnable(now, 500));      t2.start();    }      static public class MyRunnable implements Runnable {      private final Date date;      private final int  count;        public MyRunnable(final Date date, final int count) {        this.date  = date;        this.count = count;      }        public void run() {        try { Thread.sleep(count); } catch (InterruptedException e) { }        long time = new Date().getTime() - date.getTime();        System.out.println("Runtime = " + time);      }    }  }  

You should see that each runnable sleeps for 500 msec, but instead you get the wrong time information. If you change the constructor to make a defensive copy:

    public MyRunnable(final Date date, final int count) {        this.date  = new Date(date.getTime());        this.count = count;      }  

then you get the correct time information. This is a trivial example. You don't want to have to debug a complicated example.

NOTE: A common result of failure to properly manage state is a ConcurrentModificationException when iterating over a collection.

Should you code defensively? If you can guarantee that the same small team of expert programmers will always be the ones writing and maintaining your project, that they will continuously work on it so they retain memory of the details of the project, that the same people will work on it for the lifetime of the project, and that the project will never become "large," then perhaps you can get away with not doing so. But the cost to defensive programming is not large except in the rarest of cases -- and the benefit is large. Plus: defensive coding is a good habit. You don't want to encourage the development of bad habits of passing mutable data around to places that shouldn't have it. This will bite you some day. Of course, all of this depends on the required uptime of your project.


Solution:3

For both of these issues, the point is explicit control of state. It may be that most of the time you can "get away" without thinking about these things. This tends to be less true as your application gets larger and it gets harder to reason about state and how it propagates among objects.

You've already mentioned a major reason why you'd need to have control over this - being able to use the data safely while another thread was accessing it. It's also easy to make mistakes like this:

class A {     Map myMap;  }      class B {     Map myMap;     public B(A a)     {          myMap = A.getMap();//returns ref to A's myMap     }      public void process (){ // call this and you inadvertently destroy a             ... do somethign destructive to the b.myMap...        }  }  

The point is not that you always want to clone, that would be dumb and expensive. The point is not to make blanket statements about when that sort of thing is appropriate.


Solution:4

I've used Clone() to save object state in the user's session to allow for Undo during edits. I've also used it in unit tests.


Solution:5

I can think of one situation where clone is much preferable to copy constructors. If you have a function which takes in an object of type X and then returns a modified copy of it, it may be preferable for that copy to be a clone if you want to retain the internal, non-X related information. For example, a function which increments a Date by 5 hours might be useful even if it was passed an object of type SpecialDate. That said, a lot of the time using composition instead of inheritance would avoid such concerns entirely.


Solution:6

I don't like the clone() method, because there is always a type-cast needed. For this reason I use the copy-constructor most of the time. It states more clearly what it does (new object) and you have much control about how it behaves, or how deep the copy is.

At my work we don't worry about defensive programming, although that is a bad habbit. But most of the time it goes ok, but I think I am going to give it a closer look.


Solution:7

One thing I'm alwyas missing at a "defensive copy discussion" is the performance aspect. That aiscussion is IMHO a perfect example of performance vs readability/security/robustness.

Defence copies are great for ropbust ode. But if you use it in a time critical part of your app it can be a major performance issue. We had this discussion recently where a data vector stored its data in a double[] values. getValues() returned values.clone(). In our algorithm, getValues() was called for a lot of different objects. When we were wondering, why this simple piece of code took so long to execute, we inspected the code - replaced the return values.clone() with return values and suddenly our total execution time was lowered to less than 1/10 of the original value. Well - I don't need to say that we chose to skip the defensiveness.

Note: I'm not again defensive copies in general. But use your brain when clone()ing!


Solution:8

I have started using the following practice:

  1. Create copy constructors in your classes but make them protected. The reason for this is that creating objects using the new operator can lead to various issues when working with derived objects.

  2. Create a Copyable interface as follows:

       public interface Copyable<T> {              public T copy();       }  

Have the copy method of classes implementing Copyable call the protected copy constructor. Derived classes can then call super.Xxx(obj_to_copy); to leverage the base class copy constructor and adding additional functionality as required.

The fact that Java supports covariant return type makes this work. Derived classes simply implement the copy() method as appropriate and return a type-safe value for their particular class.


Note:If u also have question or solution just comment us below or mail us on toontricks1994@gmail.com
Previous
Next Post »