Hi Jacques
A lack of results won't cause the delegator to throw an exception,
assuming the model entity and the query were sound the exception would
come from a problem actually querying the database or table.
Regards
Scott
2008/8/17 Jacques Le Roux <[EMAIL PROTECTED]>:
> Maybe this was done to allow cases where an invoice has any tax items ? But
> then the message is wrong, it is
> Debug.logError(e, "Trouble getting InvoiceItem list", module);
> should be Debug.logError(e, "Trouble getting tax InvoiceItem list",
> module);
>
> My 2 cents
>
> Jacques
>
> From: "Scott Gray" <[EMAIL PROTECTED]>
>>
>> Thanks David, I guess it's pretty subjective and needs to be looked at
>> on a larger scale rather than just examining small chunks of code. I
>> might put that to one side for now unless I come across anything that
>> looks like it might obviously cause issues.
>>
>> One example I came across that did concern me was in the
>> InvoiceWorker.getInvoiceTaxTotalBd() method where if it has trouble
>> pulling up the InvoiceItems it just logs an error and returns zero
>> tax. Any opinions (anyone) on whether it is a cause for concern and
>> if so what would be the correct approach? Throw the
>> GenericEntityException?
>>
>> Thanks
>> Scott
>>
>> 2008/8/15 David E Jones <[EMAIL PROTECTED]>:
>>>
>>> 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.
>>>>>>
>>>>>
>>>
>>>
>>
>
