Effective Use of Java Reflection - is this a hack, or is this standard practice?

Question

Hey, long time listener first time caller, and I'm asking a question related to Java reflection, and how easy it lends itself to apparently ugly coding. The following method attempts to take two similar objects (one object having all of the fields of the other object, and then some) and compare the two of them for equality. It will (allegedly) return true if the getters that the objects share are equal, and will return false if they are not equal.

public boolean validateArchive( Object record, Object arcRecord ) throws IllegalAccessException, InvocationTargetException, NoSuchMethodException
{
    log.debug( record.getClass().toString() );

    Object methodValue;
    Object arcMethodValue;

    for ( Method method : record.getClass().getMethods() )
    {
        if ( method.getTypeParameters().length == 0 && method.getName().startsWith( "get" ) && !method.getName().startsWith( "getClass" ) )
        {
            methodValue = method.invoke( record );
            arcMethodValue = arcRecord.getClass().getMethod( method.getName() ).invoke( arcRecord );

            log.debug( "Method name: " + method.getName() );
            log.debug( "Archive value: " + arcMethodValue );
            log.debug( "Object value: " + methodValue );

            if ( arcMethodValue != null && methodValue != null && !arcMethodValue.equals( methodValue ) )
            {
                return false;
            }
            else
            {
                if ( arcMethodValue == null && methodValue != null || methodValue == null && arcMethodValue != null )
                {
                    return false;
                }
            }
        }
    }

    return true;
}

This method does what I expect it to do in the unit tests, but it looks ugly, and feels wrong (I'm particularly not a fan of the nested 'if'). I was just hoping for some pointers on how to do this more effectively/efficiently. If I've broken some kind of posting rule, feel free to correct me, I am eager to learn, etc.

Answer 1

For this particular task I would recommend implementing the equals method in the classes that will be compared, unless you don't have that option (for example, if you don't have the source code for the original class). IDE's like IntelliJ provide support for the "equals" and "hashcode" methods creation (in IntelliJ, you provide the fields that will be compared and which fields can be null or not). For this particular case, I would say go with these tools.

There is one case where I think that an implementation using Reflection would be useful - in a unit test, if you want to throw an assertion error if the equality is no true, you can actually throw an assertion for the exact field that is failing, and not just a generic "object is not the same" assertion error - even in this case, I would perform an equals after my original validation just to be sure that the objects are the same, or at least that the equals method is implemented and working properly.

PS: If you want to get rid of all this coding check the Beans Common library; PS 2: Reflection is not bad, and it is used everywhere where you don't have an explicit code call - Spring configuration files, for example. Just don't abuse it.

Answer 2

As you are looking only for "properties" you could use commons beanutils and more precisely this class...

(I guess you cannot implement .equals because your objects don't share the same type...)

Answer 3

I believe you're testing for unnecessary cases in your if statements, and can reduce them to this:

if ( arcMethodValue == null ) {
    if ( methodValue != null) {
        return false;
    }
} else if ( !arcMethodValue.equals( methodValue ) ) {
    return false;
}

Also, you're calling method.getName() 4 times, you could create a variable and then use it.

Answer 4

Your return code is unnecessarily complicated. The standard idiom for comparison of reference types is this:

  (field1 == null) ? (field2 == null) : field1.equals(field2);

This is much clearer and is pretty standard (Effective Java 2nd Edition, page 43).

---

Update:

There was a confusion since earlier I had written return [idiom]. Yes, in this case a return would not be what the original poster want. He would want if ![idiom] return false; instead. My point is that the [idiom] works, and it's much better.