ofbiz-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jacques Le Roux (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (OFBIZ-8341) Fix Default or Empty Catch block in Java and Groovy files
Date Tue, 05 Sep 2017 07:23:00 GMT

    [ https://issues.apache.org/jira/browse/OFBIZ-8341?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16153202#comment-16153202
] 

Jacques Le Roux commented on OFBIZ-8341:
----------------------------------------

Also it's worth to note here that arguably the same kind of exception handling that I did
in r1807240 is done in getShippableTotal() (even worse since in a loop):
{code}
    public BigDecimal getShippableTotal(String shipGroupSeqId) {
        BigDecimal shippableTotal = ZERO;
        List<GenericValue> validItems = getValidOrderItems(shipGroupSeqId);
        if (validItems != null) {
            for (GenericValue item : validItems) {
                GenericValue product = null;
                try {
                    product = item.getRelatedOne("Product", false);
                } catch (GenericEntityException e) {
                    Debug.logError(e, "Problem getting Product from OrderItem; returning 0",
module);
                    return ZERO;
                }
                if (product != null) {
                    if (ProductWorker.shippingApplies(product)) {
                        shippableTotal = shippableTotal.add(OrderReadHelper.getOrderItemSubTotal(item,
getAdjustments(), false, true)).setScale(scale, rounding);
                    }
                }
            }
        }
        return shippableTotal.setScale(scale, rounding);
    }
{code}
Theoretically it's also better here to throw an exception rather than hiding it by returning
an unknow value (here ZERO, we already spoke about this "ZERO" BTW)

I say theoretically because as Scott stated in his message on dev ML
bq. Also worth noting the reality in this case, if your system is throwing EntityExceptions
from a simple db read then whatever you're trying accomplish is already going to error out
somewhere along the chain before or after this method is called.
But this should not prevent us to write the best possible code, even if it's theoretically
in case of EntityException (something worse is happening underneath anyway)

> Fix Default or Empty Catch block in Java and Groovy files
> ---------------------------------------------------------
>
>                 Key: OFBIZ-8341
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-8341
>             Project: OFBiz
>          Issue Type: Bug
>          Components: ALL COMPONENTS
>            Reporter: Harsh Vijaywargiya
>            Assignee: Jacques Le Roux
>             Fix For: Upcoming Release
>
>         Attachments: OFBIZ-8341- OrderReadHelper.patch
>
>
> 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.
> As of now I found following classes with such patterns - 
> InventoryServices.java
> FreeMarkerWorker.java
> QRCodeEvents.java
> QRCodeServices.java



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message