HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Recursively using reflection to merge fields

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
reflectionmergefieldsrecursivelyusing

Problem

I'm using the Observer pattern to notify my UI that the object they're representing has changed. Also, I'm refreshing this object from the interwebs. Therefore, I'm ending up with two instances representing the same object. One with old values, one with refreshed values.

I have written this util class that recursively merges all fields from the refreshed instance into the original instance (Full Gist). I am wondering if and how I can optimize this, and whether I've forgotten something.

The code works for some simple use cases.

```
public class MergeUtils {

/**
* Recursively merges the fields of the provider into the receiver.
*
* @param receiver the receiver instance.
* @param provider the provider instance.
*/
public static void merge(final T receiver, final T provider) {
Field[] fields = receiver.getClass().getDeclaredFields();
for (Field field : fields) {
try {
Object receiverObject = field.get(receiver);
Object providerObject = field.get(provider);

if (receiverObject == null || providerObject == null) {
/ One is null /
field.setAccessible(true);
field.set(receiver, providerObject);
} else if (field.getType().isAssignableFrom(Collection.class)) {
/ Collection field /
//noinspection rawtypes
mergeCollections((Collection) receiverObject, (Collection) providerObject);
} else if (field.getType().isPrimitive() || field.getType().isEnum() || field.getType().equals(String.class)) {
/ Primitive, Enum or String field /
field.setAccessible(true);
field.set(receiver, providerObject);
} else {
/ Mergeable field /
merge(receiverObject, providerObject);
}
} catch (

Solution

if (receivers.isEmpty() && providers.isEmpty()) {
        return;
    }

    if (providers.isEmpty()) {
        receivers.clear();
        return;
    }


You can remove the first if statement, it's not necessary. Clearing a list that has no elements is almost free anyway.

for (T provider : providers) {
            if (idField.get(receiver).equals(idField.get(provider))) {
                found = true;
            }
        }


You could break out of the for loop here. No need to go over the rest of the providers once you've found a match.

field.getType()


You have a lot of those in your merge function. How about storing it in a temporary variable to increase readability?

Code Snippets

if (receivers.isEmpty() && providers.isEmpty()) {
        return;
    }

    if (providers.isEmpty()) {
        receivers.clear();
        return;
    }
for (T provider : providers) {
            if (idField.get(receiver).equals(idField.get(provider))) {
                found = true;
            }
        }
field.getType()

Context

StackExchange Code Review Q#51451, answer score: 2

Revisions (0)

No revisions yet.