CODEWITHSUNDEEP
javaconstructorcopy
qkad
qkad

Reputation: 53

Java Copy Constructor ArrayLists

I am trying to create a copy constructor considering the object is mutable. My copy constructor is wrong; I can't seem to figure out what I am doing wrong.

Please do NOT tell me to use clone(). How would I complete the copy constructor in this situation? I am new to Java and would really appreciate any help.

public class MyList {


public ArrayList<Cool> people;

/**
 * "people" variable as a new (empty) ArrayList of Cool objects.
 */
public MyPersonList()
{
    people = new ArrayList<Cool>(0);    
}


/**
 * A copy constructor which makes the right kind of copy considering
 * a Cool is mutable.
 */
public MyList(MyList other) 
{
    people = new ArrayList<Cool>(); 

    for(Cool p:people)
    {   
        people.add(p);
    }

}

Upvotes: 5

Views: 25057

Answers (3)

krypton-io
krypton-io

Reputation: 713

public MyList(MyList other) 
{
    people = new ArrayList<Cool>(); 

    for(Cool p:people)
    {   
        people.add(p);
    }

}

change it to : 

public MyList(MyList other) 
{
    people = new ArrayList<Cool>(); 

    for(Cool p : other.people)
    {   
        people.add(p);
    }

}

Upvotes: 2

Lone nebula
Lone nebula

Reputation: 4878

Note: Cloning the lists, isn't the same as cloning the elements in the list.

None of these approaches work the way you want them to:

//1
people = new ArrayList<Cool>(other.people);

//2
people = new ArrayList<Cool>();
for(Cool p : other.people) {
    people.add(p);
}

The approaches above will fill people such that it contains the same elements as other.people.

However, you don't want it to contain the same elements. You want to fill it with clones of the elements in other.people.

The best approach would be something like this:

people = new ArrayList<Cool>(other.people.size());
for(Cool p : other.people) {
    people.add((Cool)p.clone());
}

Make sure Cool implements Cloneable. Override clone() if necessary.

Upvotes: 12

Jack
Jack

Reputation: 133609

Simply: you are iterating over people but you should iterate over other.people variable.

Just a note: ArrayList already provides a constructor to add all items of another collection:

ArrayList(Collection<? extends E> c)

so:

people = new ArrayList<Cool>(other.people);

is enough.

Upvotes: 5

Related Questions