What I understand is that the patch is essentially changing the signature of most methods to throw an exception. On a first glance this seems to be putting the code in a worst state because now you're adding complexity for the caller to figure out how to handle these exceptions.
What is the purpose of this change? What is the gain? On Fri, Sep 8, 2017 at 12:26 PM, Michael Brohl <[email protected]> wrote: > Thanks, Jacques. > > Regards, > > Michael > > Am 08.09.17 um 10:54 schrieb Jacques Le Roux: > >> No worries Michael, >> >> I can wait a week more >> >> Jacques >> >> >> Le 08/09/2017 à 10:42, Michael Brohl a écrit : >>> >>> I disagree, not on the patch itself but on the time frame you give us to >>> review and think about it. >>> >>> I see no reason to put pressure on this issue. >>> >>> Regards, >>> >>> Michael >>> >>> >>> Am 08.09.17 um 10:02 schrieb Jacques Le Roux: >>>> >>>> If nobody disagree my last comments at OFBIZ-8341 I will commit the >>>> "OFBIZ-8341- OrderReadHelper.patch" this weekend >>>> >>>> Jacques >>>> >>>> >>>> Le 05/09/2017 à 07:31, Jacques Le Roux a écrit : >>>>> >>>>> Yes thanks Michael, >>>>> >>>>> I agree with Scott about rather throwing an exception >>>>> >>>>> Jacques >>>>> >>>>> >>>>> Le 04/09/2017 à 21:28, Michael Brohl a écrit : >>>>>> >>>>>> Hi Jacques, >>>>>> >>>>>> I think directly returning the result inside the catch block changes >>>>>> the logic of the code (the adjustments are not added). >>>>>> >>>>>> Please have another look. >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Michael >>>>>> >>>>>> >>>>>> Am 04.09.17 um 17:12 schrieb [email protected]: >>>>>>> >>>>>>> Author: jleroux >>>>>>> Date: Mon Sep 4 15:12:23 2017 >>>>>>> New Revision: 1807240 >>>>>>> >>>>>>> URL: http://svn.apache.org/viewvc?rev=1807240&view=rev >>>>>>> Log: >>>>>>> Fixed: Fix Default or Empty Catch block in Java and Groovy files >>>>>>> (OFBIZ-8341) >>>>>>> >>>>>>> In many Java and Groovy files we have auto generated catch blocks or >>>>>>> empty catch >>>>>>> blocks. >>>>>>> To avoid such exception swallowing this should be improved to at >>>>>>> least log the >>>>>>> error and also return error in case of service. >>>>>>> >>>>>>> jleroux: I can't see what we could do more here, unlikely anyway >>>>>>> >>>>>>> Modified: >>>>>>> >>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReadHelper.java >>>>>>> >>>>>>> Modified: >>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReadHelper.java >>>>>>> URL: >>>>>>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReadHelper.java?rev=1807240&r1=1807239&r2=1807240&view=diff >>>>>>> >>>>>>> ============================================================================== >>>>>>> --- >>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReadHelper.java >>>>>>> (original) >>>>>>> +++ >>>>>>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderReadHelper.java >>>>>>> Mon Sep 4 15:12:23 2017 >>>>>>> @@ -2414,10 +2414,13 @@ public class OrderReadHelper { >>>>>>> List<GenericValue> workOrderItemFulfillments = >>>>>>> null; >>>>>>> try { >>>>>>> workOrderItemFulfillments = >>>>>>> orderItem.getDelegator().findByAnd("WorkOrderItemFulfillment", >>>>>>> UtilMisc.toMap("orderId", orderItem.getString("orderId"), >>>>>>> "orderItemSeqId", >>>>>>> orderItem.getString("orderItemSeqId")), null, true); >>>>>>> - } catch (GenericEntityException e) {} >>>>>>> + } catch (GenericEntityException e) { >>>>>>> + Debug.logError(e, module); >>>>>>> + return result; >>>>>>> + } >>>>>>> if (workOrderItemFulfillments != null) { >>>>>>> Iterator<GenericValue> iter = >>>>>>> workOrderItemFulfillments.iterator(); >>>>>>> - if (iter.hasNext()) { >>>>>>> + if (iter.hasNext()) { >>>>>>> GenericValue WorkOrderItemFulfillment = >>>>>>> iter.next(); >>>>>>> GenericValue workEffort = null; >>>>>>> try { >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>> >> >
