banetl banetl - 1 year ago 114
Java Question

Generic object comparator field by field

Just for fun I wanted to try to implement a field by field, generic object comparator and this is what I did :

private Boolean isEqualFiedByField(Object o1, Object o2){
if (o1 == null || o2 == null || o1.getClass() != o2.getClass())
return false;

ObjectMapper mapper = new ObjectMapper();
Boolean result = true;
Map map1 = mapper.convertValue(o1, Map.class);
Map map2 = mapper.convertValue(o2, Map.class);

for (Object field : map1.keySet()) {
String fieldName = field.toString();
if (map1.get(fieldName) != null && map2.get(fieldName) != null)
result &= map1.get(fieldName).toString().equals(map2.get(fieldName).toString());
result &= (map2.get(fieldName) == map1.get(fieldName));
return result;

Is there anyway to improve this code ? Make it cleaner, faster or treat edges cases I forgot ?

Answer Source

Your current code uses ObjectMapper, you could also do this using reflection and not depend on any library. Not sure that's better, but something to consider.

I always put braces around blocks, even one-liners. You might later want to add a line to your if block and forget to add the braces.

You chose to handle the case with two null arguments by returning false. Is that a deliberate decision? You might want to put some JavaDoc on your method explaining this.

I think you could split your method into at least 3 parts, already indicated by empty lines in your current code. These parts do different things so could be handled in separate methods.

You are calling map1.get(fieldName) three times in your code (also map2). I would call it only once and assign the value to a local variable.

If you can get ObjectMapper (I don't know the class) to return a Map<String, Object> you can avoid all the toString calls later in the code.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download