How to avoid instanceof in copy constructor for inherited java clasess

Question

I am writing a copy constructor for cloning an object. When a class has a refernce to an object which is further inhertited by few other classes.

class Person
{
    String name;
    Address address;
 }

class HomeAdress extends Address
{
}
class OfficeAdress extends Address
{
}

Now in copy constructor for Person, to decide which Address object is to be instiated I have to use instanceof.

public Person(Person p)
{
    name = p.name;
    if(p.address instanceof HomeAddress)
    {
        address = new HomeAddress((HomeAddress) address);
    }else if(p.address instanceof OfficeAddress)
    {
        address = new OfficeAddress((OfficeAddress) address);
    }
}

So the basic problem with this as when new type of address is added to the model. I will have to add a check for the same in Person copy constructor. Is there way to avoid instanceof check to instantiate correct address object. Can I use refelction to avoid instanceof from the code?

Not an answer to your actual question, but you should use `address = new HomeAddress((HomeAddress) p.address);` (you forgot the `p.` twice). — jlordo, Mar 01 '13 at 14:11
I had similar case and solved as in the link. http://stackoverflow.com/questions/13450953/create-an-instance-within-abstract-class-using-reflection . hope it is helpful public class Derived extends Base { public static void main(String ... args) { System.out.println(new Derived().createInstance()); } } abstract class Base { public Base createInstance() { //using reflection try { return getClass().asSubclass(Base.class).newInstance(); } catch (Exception e) { throw new AssertionError(e); } } } — Ahmet Karakaya, Mar 01 '13 at 14:14
The best way is o avoid the copy constructor too. I've never written one in sixteen years of Java. Why do you think you need one? — user207421, Mar 01 '13 at 22:26

score 8 · Answer 1 · answered Mar 01 '13 at 14:11

8

You should delegate the responsibility for copying the address to the Address class. Whether you implement Cloneable or not, put a clone() method in Address, and then you can (if you need custom handling) override it in each specific Address subclass. Then in your Person copy constructor you just need:

this.address = p.address.clone();

answered Mar 01 '13 at 14:11

Jon Skeet

1,421,763
867
9,128
9,194

http://stackoverflow.com/questions/5092540/accessing-clone-from-java-lang-object http://stackoverflow.com/questions/2427883/clone-vs-copy-constructor-which-is-recommended-in-java also there are few othere places where cloning was not recemonded. So I decided with copy cosntructor. Thats why I am looing for solution other than cloning. Thanks for the help. – kunal Mar 01 '13 at 14:15
@kunal: Well you can use just a copy constructor for `Person` but still make `Address` cloneable. It looks like you've looked at the disadvantages of cloning but not considered any of the *advantages*, or ways of mitigating the disadvantages. – Jon Skeet Mar 01 '13 at 14:23
2

@kunal: I'm intrigued as to why you've accepted an answer which is effectively the same as mine, when you've disagreed with my answer. (The concept of being cloneable is separate from whether or not you actually implement the `Cloneable` interface and use `clone()` from `java.lang.Object`.) – Jon Skeet Mar 01 '13 at 14:38
3

There is nothing wrong with `clone` when implemented correctly. Your struggles with copy constructor is the exact reason why you should use `clone`. – Steve Kuo Mar 01 '13 at 17:05
@JonSkeet: I am not an expert but with the minimal reading which I did and the post which I referred in my first comment, keeping me away from accepting your answer. With method abstraction approach defined in the second answer made it pretty clear what child needs to do. – kunal Mar 01 '13 at 17:14
1

@kunal: But the "manual" version is exactly what I was talking about. Fundamentally you're still cloning the Address object, and it's the Address object which is responsible for doing so. Personally I *would* use the default implementation unless I had a reason not to, but that implementation part is a separate decision. Please note that the accepted answer makes an unnecessary (and unhelpful) string copy - I would urge you *not* to do that. – Jon Skeet Mar 01 '13 at 17:17

shuangwhywhy · Accepted Answer · 2013-03-01T14:21:48.100

-2

First, I guess it is a typo:

address = new HomeAddress((HomeAddress) p.address);

Second, instead of casting the object, you can define a copy() method in Address class, (could be) abstract:

abstract class Address {
    abstract Address copy ();
}

And you implement the method in each of the subclasses of Address, then you can call it in the constructor of Person:

public Person(Person p)
{
    name = new String(p.name);
    address = p.address.copy();
}

edited Mar 01 '13 at 14:21

answered Mar 01 '13 at 14:15

shuangwhywhy

5,475
2
18
28

1

Why are you calling `new String(p.name)`? Why not just copy the string reference? After all, strings are immutable. – Jon Skeet Mar 01 '13 at 14:47
@JonSkeet Kunal wants to make a copy of Person, but he only cloned `Person` itself and `address`, `name` is not cloned however. Of course using the same `name` won't make any difference, but I just wanted to make a "real" copy, that's all. And you are correct, copying a string is not usually necessary. – shuangwhywhy Mar 01 '13 at 15:00
3

Not only is it not necessary - it's actively harmful, as it takes both processor and CPU. Doing this in an answer with *no* explanation could easily lead to the OP copying it blindly and falling into a bad habit. It's important to understand that when cloning something, it's fine to perform simple reference copies for immutable types. – Jon Skeet Mar 01 '13 at 15:13

How to avoid instanceof in copy constructor for inherited java clasess

2 Answers2