As a generality it is important that whenever an exception is caught
it is handled so that errors don't just "disappear...".
However, there are different valid ways of handling an exception and
sometimes it is best to just log an error or warning and move on,
especially when the code is isolated such that passing it back to the
caller doesn't make sense, or it doesn't affect program flow or
anything else that might be done, but it is good to know what happened
if anyone digs into a log anyway.
One way or another the catch clause should:
1. throw another exception
2. return a message (for services and such)
3. add a message to the request or context (for events and such)
4. log the error and move on
-David
On Aug 14, 2008, at 3:08 AM, Scott Gray wrote:
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.
