Doing cloning properly in Java is way harder than it should be. I thought I knew all the traps, but recently found yet another. In a nutshell, making inner classes cloneable is almost always a bad idea. The reason is that inner classes have an internal reference to an instance of their enclosing class, which is implicitly final and can’t be changed as part of a clone. For example:

classOuterimplementsCloneable{inti=0;Innerinner=newInner();publicObjectclone(){try{finalOuterclone=(Outer)super.clone();clone.inner=(Inner)inner.clone();// A.// Cloned inner still refers to this Outer instancereturnclone;}catch(finalCloneNotSupportedExceptione){}}classInnerimplementsCloneable{intj=0;publicObjectclone(){try{returnsuper.clone();// B. WARNING! - this does the Wrong Thing}catch(finalCloneNotSupportedExceptione){}}voidinc(){i++;j++;}}}

This code is attempting to do a deep clone of an Outer instance, which requires a deep clone of the Inner it references. The problem occurs at B, because the object returned by super.clone refers to the same Outer instance as the original. So if you do this:

Outerouter1=newOuter();Outerouter2=(Outer)outer1.clone();outer2.inner.inc();// will change i in outer1 instead of outer2System.out.println(outer1.i);System.out.println(outer2.i);

You get:

10

Because the cloned outer2.inner has an internal reference to the original outer1 instance. There may be cases where you want it to work that way, but it seems very unlikely. So what to do? Calling super.clone() from an inner class is basically broken, and there’s no way to fix it. If you’re able to make the inner class final, this can be avoided relatively easily by using a copy constructor to clone the inner class instead of Object.clone:

Check out the change at line C, it’s actually valid syntax. It’s called a “qualified” new, where you explicitly specify the enclosing instance of the new inner class instance.

Using copy constructors for cloning is only OK for final classes, because they aren’t polymorphic. If we had sublcasses of Inner involved here, then it gets harder - Object.clone is the only easy way to get the right class instantiated. IntelliJ IDEA will actually give a warning on line C for this reason.

Another, possibly better, solution is to make Inner a standard (non-inner) class with an explicit, non-final reference to its Outer instance. Basically, do the inner class stuff by hand. Because the reference to the outer class isn’t final, it can be fixed up before returning from clone(). Of course, this lacks the convenience that was the motivation for inner classes in the first place.