[jira] [Comment Edited] (OFBIZ-11827) Merge identical catch blocks in single catch block

[Nicolas Malin (Jira)]

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

Jacques Le Roux edited comment on OFBIZ-11827 at 6/25/20, 11:08 AM:
--------------------------------------------------------------------

Hi Pawan,

I did a complete review up to half of the patch, then a cusory review, sounds good to me.

Is that OK (did not check more)?


{noformat}
--- applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java
+++ applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java
@@ -811,8 +811,6 @@ public class DataResourceWorker  implements org.apache.ofbiz.widget.content.Data
                     MacroFormRenderer renderer = new MacroFormRenderer(formrenderer, request, response);
                     FormRenderer formRenderer = new FormRenderer(modelForm, renderer);
                     formRenderer.render(out, context);
-                } catch (SAXException | ParserConfigurationException e) {
-                    throw new GeneralException("Error rendering Screen template", e);
                 } catch (TemplateException e) {
                     throw new GeneralException("Error creating Screen renderer", e);
                 } catch (Exception e) {
{noformat}

Also I got 3 hunks rejected:

EntitySyncServices.java.rej
{noformat}
@@ -565,14 +559,13 @@
                         // store the value(s)
                         Map<String, Object> storeResult = dispatcher.runSync("storeEntitySyncData", storeContext);
                         if (ServiceUtil.isError(storeResult)) {
-                            throw new Exception(ServiceUtil.getErrorMessage(storeResult));
+                            throw new GenericServiceException(ServiceUtil.getErrorMessage(storeResult));
                         }

                         // TODO create a response document to send back to the initial sync machine
-                    } catch (GenericServiceException gse) {
-                        return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityExtUnableToLoadXMLDocument", UtilMisc.toMap("entitySyncId", entitySyncId, "startTime", startTime, "errorString", gse.getMessage()), locale));
-                    } catch (Exception e) {
-                        return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityExtUnableToLoadXMLDocument", UtilMisc.toMap("entitySyncId", entitySyncId, "startTime", startTime, "errorString", e.getMessage()), locale));
+                    } catch (GenericServiceException | IOException | ParserConfigurationException | SAXException | SerializeException gse) {
+                        return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityExtUnableToLoadXMLDocument"
+                                , UtilMisc.toMap("entitySyncId", entitySyncId, "startTime", startTime, "errorString", gse.getMessage()), locale));
                     }
                 }
             }
{noformat}

WebToolsServices.java.rej
{noformat}
@@ -197,8 +197,6 @@
                 }
             } catch (GenericServiceException gsex) {
                 return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityImportParsingError", UtilMisc.toMap("errorString", gsex.getMessage()), locale));
-            } catch (Exception ex) {
-                return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityImportParsingError", UtilMisc.toMap("errorString", ex.getMessage()), locale));
             }
         } else {
             messages.add(UtilProperties.getMessage(resource, "EntityImportNoXmlFileSpecified", locale));
{noformat}

Update to trunk HEAD should do it, I guess


was (Author: jacques.le.roux):
Hi Pawan,

I did a complete review up to half of the patch, then a cusory review, sounds good to me.

Is that OK (did not check more)?


{noformat}
--- applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java
+++ applications/content/src/main/java/org/apache/ofbiz/content/data/DataResourceWorker.java
@@ -811,8 +811,6 @@ public class DataResourceWorker  implements org.apache.ofbiz.widget.content.Data
                     MacroFormRenderer renderer = new MacroFormRenderer(formrenderer, request, response);
                     FormRenderer formRenderer = new FormRenderer(modelForm, renderer);
                     formRenderer.render(out, context);
-                } catch (SAXException | ParserConfigurationException e) {
-                    throw new GeneralException("Error rendering Screen template", e);
                 } catch (TemplateException e) {
                     throw new GeneralException("Error creating Screen renderer", e);
                 } catch (Exception e) {
{noformat}

Also I got 3 hunks rejected:

EntitySyncServices.java.rej
{noformat}
@@ -565,14 +559,13 @@
                         // store the value(s)
                         Map<String, Object> storeResult = dispatcher.runSync("storeEntitySyncData", storeContext);
                         if (ServiceUtil.isError(storeResult)) {
-                            throw new Exception(ServiceUtil.getErrorMessage(storeResult));
+                            throw new GenericServiceException(ServiceUtil.getErrorMessage(storeResult));
                         }
 
                         // TODO create a response document to send back to the initial sync machine
-                    } catch (GenericServiceException gse) {
-                        return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityExtUnableToLoadXMLDocument", UtilMisc.toMap("entitySyncId", entitySyncId, "startTime", startTime, "errorString", gse.getMessage()), locale));
-                    } catch (Exception e) {
-                        return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityExtUnableToLoadXMLDocument", UtilMisc.toMap("entitySyncId", entitySyncId, "startTime", startTime, "errorString", e.getMessage()), locale));
+                    } catch (GenericServiceException | IOException | ParserConfigurationException | SAXException | SerializeException gse) {
+                        return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityExtUnableToLoadXMLDocument"
+                                , UtilMisc.toMap("entitySyncId", entitySyncId, "startTime", startTime, "errorString", gse.getMessage()), locale));
                     }
                 }
             }

WebToolsServices.java.rej
{noformat}
@@ -197,8 +197,6 @@
                 }
             } catch (GenericServiceException gsex) {
                 return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityImportParsingError", UtilMisc.toMap("errorString", gsex.getMessage()), locale));
-            } catch (Exception ex) {
-                return ServiceUtil.returnError(UtilProperties.getMessage(resource, "EntityImportParsingError", UtilMisc.toMap("errorString", ex.getMessage()), locale));
             }
         } else {
             messages.add(UtilProperties.getMessage(resource, "EntityImportNoXmlFileSpecified", locale));
{noformat}

Update to trunk HEAD should do it, I guess

> Merge identical catch blocks in single catch block
> ---------------------------------------------------
>
>                 Key: OFBIZ-11827
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-11827
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: ALL COMPONENTS
>    Affects Versions: Trunk
>            Reporter: Pawan Verma
>            Assignee: Pawan Verma
>            Priority: Minor
>         Attachments: OFBIZ-11827-plugins.patch, OFBIZ-11827.patch
>
>
> In Java SE 7 and later, a single catch block can handle more than one type of exception. This feature can reduce code duplication and lessen the temptation to catch an overly broad exception.
> For more details: https://docs.oracle.com/javase/8/docs/technotes/guides/language/catch-multiple.html
> Example:
> {code:java}
> catch (IOException ex) {
>     logger.log(ex);
>     throw ex;
> } catch (SQLException ex) {
>     logger.log(ex);
>     throw ex;
> }{code}
> Can be written as, which is valid in Java SE 7 and later, eliminates the duplicated code:
>  
> {code:java}
> catch (IOException | SQLException ex) {
>     logger.log(ex);
>     throw ex;
> }{code}
>  

--
This message was sent by Atlassian Jira
(v8.3.4#803005)