From: "Adrian Crum" <[email protected]>
I keep finding unnecessary calls to UtilValidate.isEmpty in the project. It seems they were added in a global S&R and committed in revision 883549.I believe this is a bad pattern to use. If I want to call a method on object A, why would I call a different method on object B that calls the method on object A? I can just call object A's method directly and avoid using object (or class) B. Not only is this indirection unnecessary, it is also confusing. From my perspective, there are serious drawbacks to using this approach: 1. Nearly every class is made dependent on UtilValidate. 2. It hurts performance. Static method calls are not free. There are places where UtilValidate.isEmpty is used when it is completely inappropriate - like for checking DOM Element attributes. They are never null, and java.lang.String has an isEmpty() method. There are other places where it is used on objects that are never null. UtilValidate.isEmpty should be used when you don't know the object's type or if it is null: Object mysteryObj = methodThatMightReturnNull(); if (UtilValidate.isEmpty(mysteryObj) { ... }
As I commented at http://svn.apache.org/viewvc?view=revision&revision=883549, what I did was to "Use UtilValidate.isNotEmpty methods instead of (obj != null) || (obj.length > 0) and (obj != null) || (obj.size > 0)" IRRW I used a simple regexp and did a complete review so no other cases should have slipped. I will do another complete review of that commit and will fix necessary hunks if any.
But maybe the issue was upstream and I simply replaced cases where (obj.size > 0) or (obj.length > 0) already did not make sense. Also Paul Foxworthy began a related work "Possible runtime errors with UtilValidate.isEmpty(Object) should be rather caught during compilation" https://issues.apache.org/jira/browse/OFBIZ-4427. I simply did not get time to review it yet... Jacques
-Adrian
