[
https://issues.apache.org/jira/browse/OFBIZ-9773?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Julian Leichert updated OFBIZ-9773:
-----------------------------------
Attachment: OFBIZ-9773_org.apache.ofbiz.product.category.ftl_bugfixes.patch
class CatalogAltUrlSeoTransform
- line 147,149 : changed "" to '' to increase perfromance
- line 169 : removed redundant toString()
class CataloUrlSeoTransform
- line 529, 640 : changed keySet to entrySet to increase performance
- line 699, 781 : changed parameter trail to null, because it' always null
(better visibility)
class OfbizCatalogAltUrlTransform
- line 143 : removed useless toString()
class SeoTransform
- added multicatch
class UrlRegexpTransform
- added multicatch
> [FB] Package org.apache.ofbiz.product.category.ftl
> --------------------------------------------------
>
> Key: OFBIZ-9773
> URL: https://issues.apache.org/jira/browse/OFBIZ-9773
> Project: OFBiz
> Issue Type: Sub-task
> Components: product
> Affects Versions: Trunk
> Reporter: Julian Leichert
> Priority: Minor
> Attachments:
> OFBIZ-9773_org.apache.ofbiz.product.category.ftl_bugfixes.patch
>
>
> CatalogAltUrlSeoTransform.java:169, DM_STRING_TOSTRING
> - Dm:
> org.apache.ofbiz.product.category.ftl.CatalogAltUrlSeoTransform$1.close()
> invokes toString() method on a String
> Calling String.toString() is just a redundant operation. Just use the String.
> CatalogUrlSeoTransform.java:531, WMI_WRONG_MAP_ITERATOR
> - WMI:
> org.apache.ofbiz.product.category.ftl.CatalogUrlSeoTransform.forwardUri(HttpServletRequest,
> HttpServletResponse, Delegator, String) 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.
> CatalogUrlSeoTransform.java:642, WMI_WRONG_MAP_ITERATOR
> - WMI:
> org.apache.ofbiz.product.category.ftl.CatalogUrlSeoTransform.forwardCategoryUri(HttpServletRequest,
> HttpServletResponse, Delegator, String) 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.
> CatalogUrlSeoTransform.java:699, NP_LOAD_OF_KNOWN_NULL_VALUE
> - NP: Load of known null value in
> org.apache.ofbiz.product.category.ftl.CatalogUrlSeoTransform.makeProductUrl(Delegator,
> ProductContentWrapper, String, String, String, String, String)
> 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).
> CatalogUrlSeoTransform.java:707, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of trail, which is known to be non-null in
> org.apache.ofbiz.product.category.ftl.CatalogUrlSeoTransform.makeProductUrl(Delegator,
> ProductContentWrapper, String, String, String, String, String)
> This method contains a redundant check of a known non-null value against the
> constant null.
> CatalogUrlSeoTransform.java:781, NP_LOAD_OF_KNOWN_NULL_VALUE
> - NP: Load of known null value in
> org.apache.ofbiz.product.category.ftl.CatalogUrlSeoTransform.makeCategoryUrl(Delegator,
> CategoryContentWrapper, String, String, String, String, String, String,
> String, String)
> 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).
> CatalogUrlSeoTransform.java:782, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of trail, which is known to be non-null in
> org.apache.ofbiz.product.category.ftl.CatalogUrlSeoTransform.makeCategoryUrl(Delegator,
> CategoryContentWrapper, String, String, String, String, String, String,
> String, String)
> This method contains a redundant check of a known non-null value against the
> constant null.
> OfbizCatalogAltUrlTransform.java:143, DM_STRING_TOSTRING
> - Dm:
> org.apache.ofbiz.product.category.ftl.OfbizCatalogAltUrlTransform$1.close()
> invokes toString() method on a String
> Calling String.toString() is just a redundant operation. Just use the String.
> SeoTransform.java:71, SIC_INNER_SHOULD_BE_STATIC_ANON
> - SIC: The class org.apache.ofbiz.product.category.ftl.SeoTransform$1 could
> be refactored into a named _static_ inner class
> This class is an inner class, but does not use its embedded reference to the
> object which created it. This reference makes the instances of the class
> larger, and may keep the reference to the creator object alive longer than
> necessary. If possible, the class should be made into a static inner class.
> Since anonymous inner classes cannot be marked as static, doing this will
> require refactoring the inner class so that it is a named inner class.
> SeoTransform.java:121, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.product.category.ftl.SeoTransform$1.close()
> 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 ...
> }
> UrlRegexpTransform.java:71, SIC_INNER_SHOULD_BE_STATIC_ANON
> - SIC: The class org.apache.ofbiz.product.category.ftl.UrlRegexpTransform$1
> could be refactored into a named _static_ inner class
> This class is an inner class, but does not use its embedded reference to the
> object which created it. This reference makes the instances of the class
> larger, and may keep the reference to the creator object alive longer than
> necessary. If possible, the class should be made into a static inner class.
> Since anonymous inner classes cannot be marked as static, doing this will
> require refactoring the inner class so that it is a named inner class.
> UrlRegexpTransform.java:121, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.product.category.ftl.UrlRegexpTransform$1.close()
> 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)
