Re: Discovery of anti-pattern

[Jacques Le Roux]

We have also 322 
   if (* != null && *.size() > 0) { 
where * is a variable name (String or Collection). Those should really better rewritten 
   if (UtilValidate.isNotEmpty(*)) {

Jacques
From: "Scott Gray" <[EMAIL PROTECTED]>
Hi All

While going through looking for the issue Adam mentioned, I'm seeing a
lot of the following:
List orl = null;
try {
   orl = delegator.findByAnd(...);
} catch (GenericEntityException e) {
   Debug.logError(e, module);
}
if (orl != null && orl.size() > 0) {
   ...
}

Shouldn't we always return an error if the delegator throws an
exception rather than continue processing?  In the example above we're
treating an exception as being equivalent to an empty result which
seems a bit funny to me...

Thanks
Scott

2008/8/13 Scott Gray <[EMAIL PROTECTED]>:
Hi Adam,

Did you have any plans to clean these up?  I don't want to start if
you're already on it.

Thanks
Scott

2008/8/13 Adam Heath <[EMAIL PROTECTED]>:
As I've been going thru more ofbiz code, adding generics, I've discovered a
bunch of the following pattern:

....
List items = delegator.findByAnd(...)
Iterator itemIt = UtilMisc.toIterator(items)
if (itemIt != null && itemIt.hasNext()) {
....

First off, delegator.findByAnd(and findList as well) never return null.
 Ever.  They will return an empty list if nothing is found.  It's only
findOne(and friends) that return null.

Because items is never null, toIterator always returns an iterator.  So
itemIt is never null.