[ 
https://issues.apache.org/jira/browse/OFBIZ-9825?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Dennis Balkir updated OFBIZ-9825:
---------------------------------
    Attachment: OFBIZ-9825_org.apache.ofbiz.content.compdoc_bugfixes.patch

class CompDocEvents:
- Line 106, 107: increased performance with the use of {{entrySet}}
- Line 117, 118: increased performance with the use of {{entrySet}}
- Line 157: removed the unnecessary null-check of {{rootDir}} because it has to 
be null at this point since it is declared null a few lines above
- Line 160: removed the unnecessary null-check of {{https}} because it has to 
be null at this point since it is declared null a few lines above
- Line 215: removed the unnecessary null-check of {{rootDir}} because it has to 
be null at this point since it is declared null a few lines above
- Line 218: removed the unnecessary null-check of {{https}} because it has to 
be null at this point since it is declared null a few lines above

class CompDocServices:
- Line 175: added a encoding charset to the {{new String}}
- Line 193: the unnecessary if-phrase could be removed, it does nothing (the 
fact that it is useless since at least 2006 made me think that it won’t be 
missed)
- Line 240: changed the catch of {{Exception}} to a multi catch to prevent 
{{RuntimeException}} to be caught and to fix this findbug-error
- Line 296:  added a encoding charset to the {{new String}}
- Line 313: the unnecessary if-phrase could be removed, it does nothing (the 
fact that it is useless since at least 2006 made me think that it won’t be 
missed)
- Line 349: changed the catch of {{Exception}} to a multi catch to prevent 
{{RuntimeException}} to be caught and to fix this findbug-error

> [FB] Package org.apache.ofbiz.content.compdoc
> ---------------------------------------------
>
>                 Key: OFBIZ-9825
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-9825
>             Project: OFBiz
>          Issue Type: Sub-task
>          Components: content
>    Affects Versions: Trunk
>            Reporter: Dennis Balkir
>            Priority: Minor
>         Attachments: 
> OFBIZ-9825_org.apache.ofbiz.content.compdoc_bugfixes.patch
>
>
> --- CompDocEvents.java:106, WMI_WRONG_MAP_ITERATOR
> WMI: 
> org.apache.ofbiz.content.compdoc.CompDocEvents.persistRootCompDoc(HttpServletRequest,
>  HttpServletResponse) makes inefficient use of keySet iterator instead of 
> entrySet iterator
> This method accesses the value of a Map entry, using a key that was retrieved 
> from a keySet iterator. It is more efficient to use an iterator on the 
> entrySet of the map, to avoid the Map.get(key) lookup.
> --- CompDocEvents.java:154, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.content.compdoc.CompDocEvents.genCompDocPdf(HttpServletRequest,
>  HttpServletResponse)
> The variable referenced at this point is known to be null due to an earlier 
> check against null. Although this is valid, it might be a mistake (perhaps 
> you intended to refer to a different variable, or perhaps the earlier check 
> to see if the variable is null should have been a check to see if it was 
> non-null).
> --- CompDocEvents.java:157, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.content.compdoc.CompDocEvents.genCompDocPdf(HttpServletRequest,
>  HttpServletResponse)
> The variable referenced at this point is known to be null due to an earlier 
> check against null. Although this is valid, it might be a mistake (perhaps 
> you intended to refer to a different variable, or perhaps the earlier check 
> to see if the variable is null should have been a check to see if it was 
> non-null).
> --- CompDocEvents.java:216, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.content.compdoc.CompDocEvents.genContentPdf(HttpServletRequest,
>  HttpServletResponse)
> The variable referenced at this point is known to be null due to an earlier 
> check against null. Although this is valid, it might be a mistake (perhaps 
> you intended to refer to a different variable, or perhaps the earlier check 
> to see if the variable is null should have been a check to see if it was 
> non-null).
> --- CompDocEvents.java:219, NP_LOAD_OF_KNOWN_NULL_VALUE
> NP: Load of known null value in 
> org.apache.ofbiz.content.compdoc.CompDocEvents.genContentPdf(HttpServletRequest,
>  HttpServletResponse)
> The variable referenced at this point is known to be null due to an earlier 
> check against null. Although this is valid, it might be a mistake (perhaps 
> you intended to refer to a different variable, or perhaps the earlier check 
> to see if the variable is null should have been a check to see if it was 
> non-null).
> --- CompDocServices.java:175, DM_DEFAULT_ENCODING
> Dm: Found reliance on default encoding in 
> org.apache.ofbiz.content.compdoc.CompDocServices.renderCompDocPdf(DispatchContext,
>  Map): new String(byte[])
> Found a call to a method which will perform a byte to String (or String to 
> byte) conversion, and will assume that the default platform encoding is 
> suitable. This will cause the application behaviour to vary between 
> platforms. Use an alternative API and specify a charset name or Charset 
> object explicitly.
> --- CompDocServices.java:193, UCF_USELESS_CONTROL_FLOW
> UCF: Useless control flow in 
> org.apache.ofbiz.content.compdoc.CompDocServices.renderCompDocPdf(DispatchContext,
>  Map)
> This method contains a useless control flow statement, where control flow 
> continues onto the same place regardless of whether or not the branch is 
> taken. For example, this is caused by having an empty statement block for an 
> if statement:
>     if (argv.length == 0) {
>     // TODO: handle this case
>     }
> --- CompDocServices.java:243, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.content.compdoc.CompDocServices.renderCompDocPdf(DispatchContext,
>  Map)
> This method uses a try-catch block that catches Exception objects, but 
> Exception is not thrown within the try block, and RuntimeException is not 
> explicitly caught. It is a common bug pattern to say try { ... } catch 
> (Exception e) { something } as a shorthand for catching a number of types of 
> exception each of whose catch blocks is identical, but this construct also 
> accidentally catches RuntimeException as well, masking potential bugs.
> A better approach is to either explicitly catch the specific exceptions that 
> are thrown, or to explicitly catch RuntimeException exception, rethrow it, 
> and then catch all non-Runtime Exceptions, as shown below:
>   try {
>     ...
>   } catch (RuntimeException e) {
>     throw e;
>   } catch (Exception e) {
>     ... deal with all non-runtime exceptions ...
>   }
> --- CompDocServices.java:299, DM_DEFAULT_ENCODING
> Dm: Found reliance on default encoding in 
> org.apache.ofbiz.content.compdoc.CompDocServices.renderContentPdf(DispatchContext,
>  Map): new String(byte[])
> Found a call to a method which will perform a byte to String (or String to 
> byte) conversion, and will assume that the default platform encoding is 
> suitable. This will cause the application behaviour to vary between 
> platforms. Use an alternative API and specify a charset name or Charset 
> object explicitly.
> --- CompDocServices.java:316, UCF_USELESS_CONTROL_FLOW
> UCF: Useless control flow in 
> org.apache.ofbiz.content.compdoc.CompDocServices.renderContentPdf(DispatchContext,
>  Map)
> This method contains a useless control flow statement, where control flow 
> continues onto the same place regardless of whether or not the branch is 
> taken. For example, this is caused by having an empty statement block for an 
> if statement:
>     if (argv.length == 0) {
>     // TODO: handle this case
>     }
> --- CompDocServices.java:347, NP_NULL_PARAM_DEREF
> NP: Null passed for nonnull parameter of java.nio.ByteBuffer.wrap(byte[]) in 
> org.apache.ofbiz.content.compdoc.CompDocServices.renderContentPdf(DispatchContext,
>  Map)
> This method call passes a null value for a non-null method parameter. Either 
> the parameter is annotated as a parameter that should always be non-null, or 
> analysis has shown that it will always be dereferenced.
> --- CompDocServices.java:354, REC_CATCH_EXCEPTION
> REC: Exception is caught when Exception is not thrown in 
> org.apache.ofbiz.content.compdoc.CompDocServices.renderContentPdf(DispatchContext,
>  Map)
> This method uses a try-catch block that catches Exception objects, but 
> Exception is not thrown within the try block, and RuntimeException is not 
> explicitly caught. It is a common bug pattern to say try { ... } catch 
> (Exception e) { something } as a shorthand for catching a number of types of 
> exception each of whose catch blocks is identical, but this construct also 
> accidentally catches RuntimeException as well, masking potential bugs.
> A better approach is to either explicitly catch the specific exceptions that 
> are thrown, or to explicitly catch RuntimeException exception, rethrow it, 
> and then catch all non-Runtime Exceptions, as shown below:
>   try {
>     ...
>   } catch (RuntimeException e) {
>     throw e;
>   } catch (Exception e) {
>     ... deal with all non-runtime exceptions ...
>   }



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

Reply via email to