0

I have a method like this:

public User getUpdatedUser(UserInfo userInfo, User user) throws ProvisioningException {
    if (!userInfo.getUserExternalId().equals(user.getImmutableId()) || !userInfo.getAccountExternalId().equals(
            getExternalAccountId(user.getAccountid())))
        throw new ProvisioningException(Response.Status.BAD_REQUEST, ProvisioningErrorCodes.INVALID_INPUT);
    if (user.getEmail() != userInfo.getEmail()) user.setEmail(userInfo.getEmail());
    if (user.getFirstName() != userInfo.getFirstName()) user.setFirstName(userInfo.getFirstName());
    if (user.getLastName() != userInfo.getLastName()) user.setLastName(userInfo.getLastName());
    if (user.getPhoneNumber() != userInfo.getPhoneNumber()) user.setPhoneNumber(userInfo.getPhoneNumber());
    if (user.getCompany() != userInfo.getCompany()) user.setCompany(userInfo.getEmail());
    if (user.getJobTitle() != userInfo.getJobTitle()) user.setJobTitle(userInfo.getJobTitle());
    if (user.getStatus() != ApiUtils.changeEnumClass(userInfo.getStatus(), DbConstants.UserStatus.class))
        user.setStatus(ApiUtils.changeEnumClass(userInfo.getStatus(), DbConstants.UserStatus.class));
    if (user.getAccountAdministratorInternalUse() != isAccountAdmin(userInfo.getRoles()))
        user.setAccountAdministratorInternalUse(isAccountAdmin(userInfo.getRoles()));
    if (user.getPodAdministratorInternalUse() != isPodAdmin(userInfo.getRoles()))
        user.setPodAdministratorInternalUse(isPodAdmin(userInfo.getRoles()));
    return user;
}

Basically, copying only those fields into user which are different. Is there a neater/cleaner way to do this in Java instead of all the if conditions?

Oleg Cherednik
  • 17,377
  • 4
  • 21
  • 35
  • 2
    I am wondering why you wan to check inequality. Is there a reason. End of the day you want user values similar to userinfo, isn't it? – NewBee Dec 19 '20 at 08:41
  • 3
    Also, you want to use `equals` for strings. – daniu Dec 19 '20 at 08:42
  • 1
    You can remove these conditions and set these values regardless of whether they match or not. Because if those values are the same then it won't make any difference and if they are different then it would update. It would certainly make it cleaner. – Mushif Ali Nawaz Dec 19 '20 at 10:55
  • Note: most of your comparisons will return false regardless of the actual values in each instance. – The Head Rush Dec 20 '20 at 15:43

3 Answers3

2

Please, consider the use of JaVers.

The library will allow you mainly to compute diffs that you can apply latter in your objects.

You can take a different approach and use a mapper library, like Mapstruct.

After installing the required dependencies, define a Mapper interface, something like:

import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.MappingTarget;
import org.mapstruct.factory.Mappers;

@Mapper
public interface UserMapper {

  UserMapper INSTANCE = Mappers.getMapper(UserMapper.class);

  void update(UserInfo userInfo, @MappingTarget User user);
}

And use it to update the properties in the target object:

UserMapper.INSTANCE.update(userInfo, user);

The library will take care of map every property in the destination object. You can tweak the mapping process as appropriate.

A lot of more elaborated, but if your information is JSON you can apply a merge patch operation over the destination object with the received JSON information. Please, review this excellent article, it is focused on Spring, but it will provide you a great background about the proposed solution.

Finally, you can follow a simpler approach and use Apache Commons BeanUtils and copyProperties between your objects.

Please, be aware that with the exception of the first solution based on JaVers, the rest of the proposed approaches will copy all properties from the source to the destination object, not only the different ones. Unless for the possible performance overhead or any unindicated side effect, this fact will make no difference in the result obtained.


Alternative approach, use it with responsability.

Manual mode (reflection).

  //user is the User instance you wish to modify
 
  Class<User> rfkClass = User.class;
  Field field = rfkClass.getDeclaredField("name"); //private final? Who cares
  field.setAccessible(true);
  field.set(user, "TheDestroyer");                      
  
  field = rfkClass.getDeclaredField("email");
  field.setAccessible(true);
  field.set(user, "chuck@norris.com");                               
  //... your changes here

  // return user;   --> same instance, now modified internally

Regardless the variables were declared as final, private, whatever, reflection just doesn't care.

aran
  • 10,978
  • 5
  • 39
  • 69
jccampanero
  • 50,989
  • 3
  • 20
  • 49
  • That JaVers is indeed very interesting. It uses the god of gods, aka reflection, as expected in order to apply its mechanism. Should be an interesting read on their src to get new ideas..... – aran Feb 22 '21 at 19:13
  • 1
    Sorry @aran, I just realized your edit and the comment. Thank you very much, I really appreciate that!! Yes, I totally agree with you. I honestly only reviewed some classes in order to understand the actual behavior of the library, but is seems very well programmed. We used it in several projects for object graph differentiation and always with success. It is highly recommended. – jccampanero Feb 22 '21 at 22:28
0

You could define an API by which both objects abide, and then use reflection to create a generic method.

interface User {
    void setFoo(String foo);

    String getFoo();
}

class UserImpl implements User { ... }

class UserInfo implements User { ... }
public class Mirrorer<T> {
    private final String[] methodNames;
    private final MethodHandle[] methodHandles;

    public Mirrorer(Class<T> interfaceClass) {
        if (!interfaceClass.isInterface()) {
            throw new IllegalArgumentException("interfaceClass must be an interface.");
        }
        Method[] methods = interfaceClass.getDeclaredMethods();
        int methodsCount = methods.length;
        methodNames = new String[methodsCount];
        methodHandles = new MethodHandle[methodsCount];
        MethodHandles.Lookup lookup = MethodHandles.lookup();
        try {
            for (int i = 0; i < methodsCount; i++) {
                Method method = methods[i];
                methodNames[i] = method.getName();
                methodHandles[i] = lookup.unreflect(method);
            }
        } catch (IllegalAccessException e) {
            throw new AssertionError();
        }
    }

    public void mirror(T from, T to) throws Throwable {
        for (int i = 0; i < methodNames.length; i++) {
            String methodName = methodNames[i];
            if (methodName.startsWith("get")) {
                MethodHandle methodHandle = methodHandles[i];
                Object fromValue = methodHandle.invoke(from);
                Object toValue = methodHandle.invoke(to);
                if (!Objects.equals(fromValue, toValue)) {
                    MethodHandle setter = getSetter(methodName.substring(3), methodHandle.type().returnType());
                    setter.invoke(to, fromValue);
                }
            }
        }
    }

    private MethodHandle getSetter(String field, Class<?> type) {
        for (int i = 0; i < methodNames.length; i++) {
            String methodName = methodNames[i];
            if (methodName.equals("set" + field)) {
                MethodHandle methodHandle = methodHandles[i];
                MethodType methodType = methodHandle.type();
                if (methodType.parameterCount() != 0 && methodType.parameterType(0).equals(type)) {
                    return methodHandle;
                }
            }
        }
        throw new AssertionError();
    }
}
Mirrorer<User> mirrorer = new Mirrorer<>(User.class);
UserInfo userInfo = ...
UserImpl user = ...
mirrorer.mirror(userInfo, user);
spongebob
  • 8,370
  • 15
  • 50
  • 83
0

Looks like you just copy all different fields from userInfo to user and you are able not to compare all the valuse, just set it all together.

public User getUpdatedUser(UserInfo userInfo, User user) throws ProvisioningException {
    if (!userInfo.getUserExternalId().equals(user.getImmutableId())
        || !userInfo.getAccountExternalId().equals(getExternalAccountId(user.getAccountid())))
        throw new ProvisioningException(Response.Status.BAD_REQUEST, ProvisioningErrorCodes.INVALID_INPUT);
    
    user.setEmail(userInfo.getEmail());
    user.setFirstName(userInfo.getFirstName());
    user.setLastName(userInfo.getLastName());
    user.setPhoneNumber(userInfo.getPhoneNumber());
    user.setCompany(userInfo.getEmail());
    user.setJobTitle(userInfo.getJobTitle());
    user.setStatus(userInfo.getStatus());
    user.setAccountAdministratorInternalUse(isAccountAdmin(userInfo.getRoles()));
    user.setPodAdministratorInternalUse(isPodAdmin(userInfo.getRoles()));
    
    return user;
}
Oleg Cherednik
  • 17,377
  • 4
  • 21
  • 35
  • With this approach all fields would be copied and it doesn't meet the original requirement - `copying only those fields which are different` – Nikolai Shevchenko Dec 19 '20 at 13:58
  • So, the result is identical objects. So no need to check that value is different, because if it same and you copy it, the result will not change. – Oleg Cherednik Dec 19 '20 at 14:00
  • I don't understand OP's intention, so I'm just guessing. Maybe setters of User class have side effects that OP wants to avoid for non-changed fields – Nikolai Shevchenko Dec 19 '20 at 14:20