Wednesday, April 2, 2008

Checking for any property in a bean

I came across a situation the other day at work where I needed to know if any property was set on a bean. There are a couple of uses cases that involve checking that a bean being used as a value object or transfer object has at least one property set before doing some heavy lifting based on the contents of the bean. The two use cases involved doing a database query or generating XML based on the bean. If the bean is empty, the query does not need to be performed or the XML does not need to be generated. The first solution is a simple if-else-if chain:
if(bean.getProp1() != null) {
    return true;
} else if(bean.getProp2() != null) {
    return true;
}  else {
    return false;
}
There are a couple of problems with this approach. First, it is just plain ugly for more than a couple of properties. This ugliness quickly translates into a high cyclomatic complexity for more than a few properties. The code also leads to maintenance bugs as it is easy to forgot to add new properties to the chain. I took a survey of a few of my coworkers and we came up with several solutions.

hashCode

If hashCode is defined to return zero (0) if none of the properties are set and a standard hash code otherwise, then hashCode makes a good candidate for checking if any property has been set. In practice, simple unit tests proved that the algorithms for setting hashCode do not lend themselves nicely to having a predictable value, like 0, for when none the properties are set. The hashCode ends up being based on the number of properties as well as the content.

Dirty Bit

This solution consists in a adding a boolean flag to the the object, anyValue. The flag is set to false and every setter would set it to true. Then a new method, hasAnyValue would simple return anyValue.
public void setProp1(Prop1 newProp1) {
  prop1 = newProp1;
  if(newProp1 != null ) anyValue = true;
}

public boolean hasAnyValue() {
   return anyValue;
}
We decided against this one for a couple of reasons. While it removes the cycolmatic complexity problem, is fast and is easy to understand, it still has the problem of a maintainer forgetting to add the assignment of anyValue to true in new setters. Also, it does not handle the case where a property is set back to null after having been set to a new value. Using a counter that is incremented and decremented would work around that problem. This method works well where only a subset of the properties need to be checked. The only the relevant setters need contain the anyValue assignment.

AOP

Using aspects to make the assignment to anyValue removes the maintenance problem of forgetting to make the assignment by adding another level of complexity to the application. If an application already makes use of aspects, this would make sense. Adding aspects for just this use case would have been swatting mosquitoes with sledge hammers.

Reflection

Another approach would be to remove the anyValue field and change the method hasAnyValue to use reflection to introspect the properties and return true if any of them is non-null. While this would work, reflection code is ugly and hard to understand.

BeanUtils

Fortunately, the nice folks at Jakarta have a Commons BeanUtils package that performs operations on beans. While I couldn't find a method that checks for any value being set, there is a method that retrieves all the properties of a bean into a map: PropertyUtils.describe Using this, the hasAnyValue method becomes:
    public static boolean hasValue(Object object) {
        Map describe;
        try {
            describe = BeanUtils.describe(object);
            for (Iterator iterator = describe.entrySet().iterator(); iterator.hasNext();) {
                Map.Entry entry = (Map.Entry) iterator.next();
                if (!"class".equals(entry.getKey()) && entry.getValue() != null) {
                    return true;
                }
            }
        } catch (IllegalAccessException e) {
            LOG.error("Failed to check hasValue 1", e);
        } catch (InvocationTargetException e) {
            LOG.error("Failed to check hasValue 2", e);
        } catch (NoSuchMethodException e) {
            LOG.error("Failed to check hasValue 3", e);
        }
        return false;
    }
This solution provided the flexibility of the reflection solution without having to maintain the reflection code. Note: I have not checked this with primitive properties. One drawback is that this solution is slower than any of the others because it reads all properties, even if all of them are non-null. If more speed is needed or if only a subset of properties need be checked, consider using the dirty bit solution.

No comments: