How to write maintainable merge() method?

Question

Let's say we have the Person entity:

class Person {
    /*
        once the id is assigned, then must not be modified! 
        assume that the id will be assigned by the ORM framework
    */
    int id;
    String givenName;
    String familyName;
}

And I we have two Persons: original Person and updated Person:

Person original = new Person("Frantisek", "Makovicka");
Person updated = new Person("Viktor", "Makovicka");

I want to merge updated Person with original Person, so I wrote the following simple method:

// return number of changed fields
public int merge(Person original, Person updated) {
    int changes = 0;

    String oldGivenName = original.givenName;
    original.givenName = updated.givenName;
    if (changed(oldGivenName, original.givenName)) changes++;

    String oldFamilyName = original.familyName;
    original.familyName = updated.familyName;
    if (changed(oldFamilyName, original.familyName)) changes++;

    return changes;
}

It works fine but I see some problems:
every time there will be added new field to the Person class, the programmer should not to forget to update the merge() method and in case the Person have really many fields then it will be difficult to maintain this method.

So my question is: is there any smarter/robust way to merge the state of the objects without using reflection feature of the language, so that you can be sure that all and only needed fields are merged? Thanks in advance!

UPD:

Originally I asked if there is the way of writing it without of using reflection but forget to say that it is not the restriction! I should also say that I had an idea to write this method with reflection + annotating "merge-able" fields with some custom annotation and then just skip fields without annotation. So, the intention of these words: "without using reflection" is to discover other maybe not so obvious solutions :)

Inspiration for this question was this functional style method: (which ensures that resource will be closed, take it only as example of not so obvious solution and safe programming)

public static void doWithResource(String name, Consumer<Resource> consumer) {
    Resource res = new Resource(name);
    consumer.accept(res);
    res.close();
}

changed is not defined. You must mean changes++. The way it is setup now, you will always get back 0 from the above method. — Inxsible, Jun 21 '17 at 14:15
You are overwriting the original with the updated, in which case you probably should be just overwriting the original pointer, not calling a merge method. If you must do this, I think you are looking for reflection. (main difference being, if these are two different classes that extend Person, should the merge care?) — Tezra, Jun 21 '17 at 14:21
You could do some reflection and look at every (maybe annotated) field of your class. — danielspaniol, Jun 21 '17 at 14:22
"without using reflection feature of the language" -- why not? — slim, Jun 21 '17 at 15:08
Would the use of [java.beans.Introspector](http://docs.oracle.com/javase/8/docs/api/java/beans/Introspector.html) to obtain [PropertyDescriptors](http://docs.oracle.com/javase/8/docs/api/java/beans/PropertyDescriptor.html) count as “using reflection”? (It does use reflection internally, of course, but the code would be a little cleaner than using reflection directly.) — VGR, Jun 21 '17 at 16:23
@VGR in the updated question I wrote that using of reflection is not the restriction. Using of java.beans.Introspector looks like "not so obvious solution" and if it accounts with all other requirements it will be accepted. I'm not familiar with that component, so I will be glad if you could write the answer. Thanks! — uanacau, Jun 21 '17 at 17:09

score 2 · Accepted Answer · answered Jun 21 '17 at 15:01

I see 3 features you need:

"id" must not be modified
a change count should be returned
any change on "updated" should be applied to "original"

Especially the first 2 requirements are a problem. I don't think that there's any library based solution which will do exactly this. However, you can write your own "merger" with some Java reflexion:

private int merge(Person p1, Person p2) throws IllegalAccessException {
    int changes = 0;
    for(Field field: Person.class.getDeclaredFields()) {
        if(!field.getName().equals("id")) {
            field.setAccessible(true);
            Object originalField = field.get(p1);
            Object updatedField = field.get(p2);
            if(!originalField.equals(updatedField)) {
                field.set(p1, updatedField);
                changes++;
            }
        }
    }
    return changes;
}

Question says "without using reflection" - I don't know why. — slim, Jun 21 '17 at 15:07
@Thomas Uhrig Thank you for your answer! I wonder if the equality operation performed on objects of raw Object type will properly handle equation of fields of types which have overriden equals(), hashCode() methods? Won't it will just perform the type comparison as it definied in Object class `public boolean equals(Object obj) { return (this == obj); }`? — uanacau, Jun 22 '17 at 09:54

score 1 · Answer 2 · answered Jun 21 '17 at 17:45

You can use java.beans.Introspector to obtain PropertyDescriptors. This is close to using reflection, and in fact the java.beans package uses reflection internally, but it’s at least a little cleaner, and will be (mostly) restricted to bean methods (get/set/is methods):

public int merge(Person original, Person updated) {
    int changes = 0;

    try {
        BeanInfo info = Introspector.getBeanInfo(Person.class, Object.class);
        for (PropertyDescriptor property : info.getPropertyDescriptors()) {
            if (property.getName().equalsIgnoreCase("id")) {
                continue;
            }

            Method get = property.getReadMethod();
            Method set = property.getWriteMethod();
            if (set == null) {
                // Ignore read-only property.
                continue;
            }

            Object oldValue = get.invoke(original);
            Object newValue = get.invoke(updated);

            set.invoke(original, newValue);

            if (changed(oldValue, newValue)) {
                changes++;
            }
        }
    } catch (IntrospectionException | ReflectiveOperationException e) {
        // We should never get here.
        throw new RuntimeException(
            "Could not update properties of " + Person.class + ": " + e, e);
    }

    return changes;
}

However, this always performs a shallow copy. If any property has a mutable type (like an array or Collection type, or a mutable object type like, in theory, Address), and if your methods do not do defensive copying of such types, the two objects will be sharing objects which will lead to frustrating, dormant bugs.

If your coders all know to perform defensive copying, or if you’re sure no one will add properties with mutable types, it’s not an issue. Otherwise, it’s going to get complicated enough that it may not be worth trying to automate the copying of properties; at the very least, you’ll need to check for an array or collection type and clone the value.

Thank you for your interesting answer! I'm looking on it and trying to grasp the advantage of this approach over clear reflection. Can you please clarify it? >and will be (mostly) restricted to bean methods (get/set/is methods) What it implies? More over I think that approach with clear reflection is more flexible, because I can for exmaple skip annotated fields or mutable fields (i.e. collections as you said) — uanacau, Jun 22 '17 at 15:12
A bean property is represented by a getter method, whose name, according to the Java Beans specification, must start with `get`, unless its return type is `boolean`, in which case it must start with either `get` or `is`. If the bean property is not read-only, it also has a setter method, whose name must start with `set`. The java.beans package abides by these rules. — VGR, Jun 22 '17 at 17:17

score 0 · Answer 3 · answered Jun 21 '17 at 14:14

0

https://commons.apache.org/proper/commons-beanutils/ might be helpful for you with its copyProperties(Object ToCopyTo, Obj ToCopyFrom)

answered Jun 21 '17 at 14:14

ZeldaZach

478
1
9
18

This will overwrite every property, including the "id" which shouldn't be modified. – Thomas Uhrig Jun 21 '17 at 14:57

How to write maintainable merge() method?

3 Answers3