search

aucd29 / cloning

Automatically exported from code.google.com/p/cloning
Other
0 stars 0 forks source link

Fast cloners should use interface not implementation #34

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
In the fast cloners the object to be cloned is passed in and type cast to an 
implementation, such as HashMap:

        final HashMap<Object, Object> m = (HashMap) t;

It should be type cast to the interface for those collections such as:

        final Map<Object, Object> m = (Map) t;

This then allows the use of the existing fast cloners for other collection 
implementations such as Hibernate's PersistentMap or PersistentSet.

Original issue reported on code.google.com by jklap...@gmail.com on 22 May 2014 at 6:28

GoogleCodeExporter commented 9 years ago 
this is purposely done this way because if say we need to clone a private 
HashMap m or a private TreeMap m then we can not use a generic class (that 
doesn't check the field type). So the fast cloner for HashMap works only for 
HashMap

Original comment by kostas.k...@googlemail.com on 21 Jun 2014 at 7:52

GoogleCodeExporter commented 9 years ago 
Sorry- not following, can you give an example that would fail? I've copied the 
existing fast cloners- replaced the implementation with the interface and 
haven't run into any issues yet.

Original comment by jklap...@gmail.com on 21 Jun 2014 at 9:03

GoogleCodeExporter commented 9 years ago 
So say you got class A { private HashMap m=... } and class B {private TreeMap 
m=....} . Currently FastClonerHashMap works correctly because it will clone A.m 
. If FastClonerHashMap was generic, it would fail to clone B.m (or it would 
have to be aware of every implementation of Map which is not possible).

In your case it would work because I suppose your class declares a Map m=...

Original comment by kostas.k...@googlemail.com on 22 Jun 2014 at 4:44

GoogleCodeExporter commented 9 years ago 
Not sure if that example is valid given that you have fast cloners for both 
HashMap and TreeMap?

Original comment by jklap...@gmail.com on 10 Sep 2014 at 2:32

GoogleCodeExporter commented 9 years ago 
say for example the FastClonerHashMap:

public class FastClonerHashMap implements IFastCloner
{
    @SuppressWarnings({ "unchecked", "rawtypes" })
    public Object clone(final Object t, final IDeepCloner cloner, final Map<Object, Object> clones) {
        final HashMap<Object, Object> m = (HashMap) t;
        final HashMap result = new HashMap();
        for (final Map.Entry e : m.entrySet())
        {
            final Object key = cloner.deepClone(e.getKey(), clones);
            final Object value = cloner.deepClone(e.getValue(), clones);

            result.put(key, value);
        }
        return result;
    }
}

notice these lines:

        final HashMap<Object, Object> m = (HashMap) t;
        final HashMap result = new HashMap();

That's safe because this is responsible for cloning only HashMap instances. By 
changing m to a Map<>, it doesn't make the fast cloner a generic Map cloner.

Original comment by kostas.k...@googlemail.com on 10 Sep 2014 at 6:39

GoogleCodeExporter commented 9 years ago 
Changing m to Map does make it a generic Map cloner-- caveat that the cloned 
object is an instance of HashMap, which may possibly be different then the 
original object type, BUT since the user is responsible for registering this 
FastCloner with specific classes other then HashMap then the user accepts the 
responsibility that the resulting cloned object is a HashMap.

Changing m to Map doesn't effect normal out-of-the-box functionality yet will 
allow a user to re-use the FastCloners with that accepted caveat.

Original comment by jklap...@gmail.com on 12 Sep 2014 at 4:15

GoogleCodeExporter commented 9 years ago 
also this line must change:

final HashMap result = new HashMap();

probably it would require an abstract class where client code can override a 
method to create the appropriate map.

Original comment by kostas.k...@googlemail.com on 12 Sep 2014 at 7:34