Override equals() and hashCode() using reflection

I wrote a utility method to override equals() using reflection. This works fine, but I wonder if this code will pass all the tests.

```
public static boolean checkObjectsEqualityFromInstance(Object currentObj,Object otherObj) throws Exception{
if(currentObj==null || otherObj==null){
return false;
}
else if(otherObj.getClass()!=null&¤tObj.getClass()!=null&&!currentObj.getClass().isInstance(otherObj)){
return false;
}
boolean result =true;
Field[] fields = otherObj.getClass().getDeclaredFields();
Field[] currentObjFields = currentObj.getClass().getDeclaredFields();

Map attriButeNameValueMap=null; /This map to store property name and its value of any of object in my calse the otherObj */
try {
attriButeNameValueMap=new HashMap<>();
for (Field field : fields) {
field.setAccessible(true);

Object value = field.get(otherObj);
attriButeNameValueMap.put(field.getName(), value);
}
for (Field field : currentObjFields) {
field.setAccessible(true);

Object value = field.get(currentObj);
if(attriButeNameValueMap.containsKey(field.getName())){
if(value instanceof Boolean){
result=areEqual((Boolean)value, (Boolean)attriButeNameValueMap.get(field.getName()));
}
else if(value instanceof Character){
result=result&&areEqual((Character)value, (Character)attriButeNameValueMap.get(field.getName()));
}
else if(value instanceof Long){
result=result&&areEqual((Long)value, (Long)attriButeNameValueMap.get(field.getName()));
}
else if(value instanceof Float){
result=result&&areEqual((Float)value, (Float)attriButeNameValueMap.get(fie

-
It ignores values in superclasses. You might want to check that too.

Anyway, don't reinvent the weel, there is a library for that! I guess EqualsBuilder.reflectionEquals from Apache Commons Lang does exactly what you want. It also has solutions to corner cases, like

• transient fields (since they are likely derived fields, and not part of the value of the Object) in a configurable way

• generated fields with $ (for inner classes).

You can also exclude field names from the comparison.

It's open-source, you can check the source for further details.

See also: Effective Java, 2nd edition, Item 47: Know and use the libraries (The author mentions only the JDK's built-in libraries but I think the reasoning could be true for other libraries too.)

-
The else keyword is unnecessary here:

if(currentObj==null || otherObj==null){
    return false;
}
else if(otherObj.getClass()!=null&¤tObj.getClass()!=null&&!currentObj.getClass().isInstance(otherObj)){
    return false;
}

The following is the same:

if (currentObj == null || otherObj == null){
    return false;
}
if (otherObj.getClass() != null && currentObj.getClass() != null && 
        !currentObj.getClass().isInstance(otherObj)) {
    return false;
}

It's called guard clause. I've also used a little bit more spacing to make it readable (easier separaton of values, comparisons, method calls).

-
Guard clauses would make this easier to follow too:

if(value instanceof Boolean){
      result=areEqual((Boolean)value, (Boolean)attriButeNameValueMap.get(field.getName()));
  }
  else if(value instanceof Character){
      result=result&&areEqual((Character)value, (Character)attriButeNameValueMap.get(field.getName()));
  }
  else if(value instanceof Long){
      result=result&&areEqual((Long)value, (Long)attriButeNameValueMap.get(field.getName()));
  }
  ...
  if(!result){
      return result;
  }

I've used a few exaplanatory variable to remove some duplication:

Object value = field.get(currentObj);
String fieldName = field.getName();
if(attriButeNameValueMap.containsKey(fieldName)){
      Object otherFieldValue = attriButeNameValueMap.get(fieldName);
      if (value instanceof Boolean) {
          if (!areEqual((Boolean) value, (Boolean) otherFieldValue)) {
              return false;
          }
      } 
      if (value instanceof Character) {
          if (!areEqual((Character) value, (Character) otherFieldValue)) {
              return false;
          }
      }

Note that it makes superfluous the result variable. The last line of the method could be simply the following, since its never modified anymore:

return true;

-
Formatting is not consistent. Indentation sometimes is two spaces, sometimes four.

-

Map attriButeNameValueMap=null; 
  /*This map to store *property name and its value of any of object in my calse the otherObj */
  try {
      attriButeNameValueMap=new HashMap<>();

B should be lowercase in the variable name (I guess it's just a typo) and you could declare it inside the try block:

I'd also rename it to otherObjectAttributeValues to make the comment unnecessary:

try {
    Map otherObjectAttributeValues = new HashMap<>();

(Effective Java, Second Edition, Item 45: Minimize the scope of local variables)

-
Consistent formatting would be great here too:

} catch (IllegalArgumentException | IllegalAccessException e) {
              // TODO Auto-generated catch block
              e.printStackTrace();
  throw e;
          }

Furthermore, TODO comments does not suggest professional work. Fix that and remove the comment.

I'd throw out the IllegalAccessException or wrap it into a RuntimeException. The IllegalArgumentException already a RuntimeException, so you don't have to put it into the method signature.

StackExchange Code Review Q#45579, answer score: 8