views:

110

answers:

4

Is cloning good practice in this case? How to do it better?

public ModelCollection startParsing() {

   return parseFeed(new ModelSpecialEntry); 
}

public ModelCollection parseFeed(ModelEntry pattern)  {

   ModelCollection modelCollection = new ModelCollection();

   while( condition ) {

     //TODO: Is cloning the best solution?
     ModelEntry model = (ModelEntry) pattern.clone();



     model.parse();

     //add this item to an collection
     modelCollection.add(model);


   }

   return modelCollection;
}
A: 

I thing, that it is like with everything else in programming: it depends on the object specyfication.

Try to make a really quick test: clone 100000 objects and instantiates the same amount of objects and check time how long it takes (System.currentTimeInMilis()). Often clone is faster.

And remember that with clone there is one problem - when adding new field etc. you need to modify clone() method too.

Jan Wegner
Oh, modify the clone method? Is this necessary? I thought this is just a method acting kind like a constructor, which is also not necessary.
OneWorld
If you don't implement `clone` in your classes you'll get a `CloneNotSupportedException` thrown when you call it.
Jon Freedman
+4  A: 

Cloning is rarely a good idea in Java. Try other techniques such as Copy constructors or Factory methods.

Wikipedia has a nice article on why clone() has many disadvantages in Java.

Using copy constructors, create a constructor that takes an instance of the current class as parameter, and copy all fields in the local class:

public class Foo {

    private String bar;
    private String baz;

    public Foo(Foo other) {
        this.bar = other.bar;
        this.baz = other.baz;
    }

}

Using factory methods, create a method that takes your object as parameter and return an object containing the same values:

public Foo copyFoo(Foo other) {
    Foo foo = new Foo();
    foo.setBar(other.getBar());
    foo.setBaz(other.getBaz());
}
Vivien Barousse
You can also have partial copy methods: `public withBar(String newBar) { return new Foo(newBar, this.baz); }`
abhin4v
+2  A: 

You could use a copy constructor instead of implementing Cloneable, but it looks like you have a hierarchy of ModelEntry classes, so using clone may be the best approach. See this question for some pointers on whats wrong with Cloneable

Jon Freedman
A: 

Clone is not a good idea as many programmers agree.

It's error-prone. You have to override clone() carefully.Forgetting invoking super.clone() is a popular bug.

卢声远 Shengyuan Lu