[
https://issues.apache.org/jira/browse/OFBIZ-9781?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Julian Leichert updated OFBIZ-9781:
-----------------------------------
Attachment:
OFBIZ-9781_org.apache.ofbiz.order.shoppingcart.product_bugfixes.patch
class ProductDisplayWorker
- added hashCode
class ProductPromoWorker
- multiple lines : removed redundant null-check
- line 383 : changed catched Exception
- line 880 : changed keySet to foreach with entrySet
- multiple lines : indexOf "" to '' (increase performance)
- line 2092 : removed useless else
- line 2147 : changed to isEmpty (correct way to check if String empty)
> [FB] Package org.apache.ofbiz.order.shoppingcart.product
> --------------------------------------------------------
>
> Key: OFBIZ-9781
> URL: https://issues.apache.org/jira/browse/OFBIZ-9781
> Project: OFBiz
> Issue Type: Sub-task
> Components: order
> Affects Versions: Trunk
> Reporter: Julian Leichert
> Priority: Minor
> Attachments:
> OFBIZ-9781_org.apache.ofbiz.order.shoppingcart.product_bugfixes.patch
>
>
> ProductDisplayWorker.java:63, BC_UNCONFIRMED_CAST
> - BC: Unchecked/unconfirmed cast from javax.servlet.ServletRequest to
> javax.servlet.http.HttpServletRequest in
> org.apache.ofbiz.order.shoppingcart.product.ProductDisplayWorker.getRandomCartProductAssoc(ServletRequest,
> boolean)
> This cast is unchecked, and not all instances of the type casted from can be
> cast to the type it is being cast to. Check that your program logic ensures
> that this cast will not fail.
> ProductDisplayWorker.java:150, BC_UNCONFIRMED_CAST
> - BC: Unchecked/unconfirmed cast from javax.servlet.ServletRequest to
> javax.servlet.http.HttpServletRequest in
> org.apache.ofbiz.order.shoppingcart.product.ProductDisplayWorker.getQuickReorderProducts(ServletRequest)
> This cast is unchecked, and not all instances of the type casted from can be
> cast to the type it is being cast to. Check that your program logic ensures
> that this cast will not fail.
> ProductDisplayWorker.java:303, SE_COMPARATOR_SHOULD_BE_SERIALIZABLE
> - Se:
> org.apache.ofbiz.order.shoppingcart.product.ProductDisplayWorker$ProductByMapComparator
> implements Comparator but not Serializable
> This class implements the Comparator interface. You should consider whether
> or not it should also implement the Serializable interface. If a comparator
> is used to construct an ordered collection such as a TreeMap, then the
> TreeMap will be serializable only if the comparator is also serializable. As
> most comparators have little or no state, making them serializable is
> generally easy and good defensive programming.
> ProductDisplayWorker.java:329, HE_EQUALS_USE_HASHCODE
> - HE:
> org.apache.ofbiz.order.shoppingcart.product.ProductDisplayWorker$ProductByMapComparator
> defines equals and uses Object.hashCode()
> This class overrides equals(Object), but does not override hashCode(), and
> inherits the implementation of hashCode() from java.lang.Object (which
> returns the identity hash code, an arbitrary value assigned to the object by
> the VM). Therefore, the class is very likely to violate the invariant that
> equal objects must have equal hashcodes.
> If you don't think instances of this class will ever be inserted into a
> HashMap/HashTable, the recommended hashCode implementation to use is:
> public int hashCode() {
> assert false : "hashCode not designed";
> return 42; // any arbitrary constant will do
> }
> ProductPromoWorker.java:117, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of productStore, which is known to be non-null in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.getStoreProductPromos(Delegator,
> LocalDispatcher, ServletRequest)
> This method contains a redundant check of a known non-null value against the
> constant null.
> ProductPromoWorker.java:382, REC_CATCH_EXCEPTION
> - REC: Exception is caught when Exception is not thrown in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.doPromotions(ShoppingCart,
> List, LocalDispatcher)
> 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 ...
> }
> ProductPromoWorker.java:507, NP_LOAD_OF_KNOWN_NULL_VALUE
> - NP: Load of known null value in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.getProductPromoUseLimit(GenericValue,
> String, Delegator)
> 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).
> ProductPromoWorker.java:507, RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE
> - RCN: Redundant nullcheck of candidateUseLimit which is known to be null in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.getProductPromoUseLimit(GenericValue,
> String, Delegator)
> This method contains a redundant check of a known null value against the
> constant null.
> ProductPromoWorker.java:567, NP_LOAD_OF_KNOWN_NULL_VALUE
> - NP: Load of known null value in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.getProductPromoCodeUseLimit(GenericValue,
> String, Delegator)
> 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).
> ProductPromoWorker.java:567, RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE
> - RCN: Redundant nullcheck of codeUseLimit which is known to be null in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.getProductPromoCodeUseLimit(GenericValue,
> String, Delegator)
> This method contains a redundant check of a known null value against the
> constant null.
> ProductPromoWorker.java:882, WMI_WRONG_MAP_ITERATOR
> - WMI:
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.prepareDeltaProductUsageInfoMap(Map,
> Map) 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.
> ProductPromoWorker.java:1413, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE
> - RCN: Nullcheck of cart at line 1501 of value previously dereferenced in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.performAction(ProductPromoWorker$ActionResultInfo,
> GenericValue, ShoppingCart, Delegator, LocalDispatcher, Timestamp)
> A value is checked here to see whether it is null, but this value can't be
> null because it was previously dereferenced and if it were null a null
> pointer exception would have occurred at the earlier dereference.
> Essentially, this code and the previous dereference disagree as to whether
> this value is allowed to be null. Either the check is redundant or the
> previous dereference is erroneous.
> ProductPromoWorker.java:1489, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
> - RCN: Redundant nullcheck of productIds, which is known to be non-null in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.performAction(ProductPromoWorker$ActionResultInfo,
> GenericValue, ShoppingCart, Delegator, LocalDispatcher, Timestamp)
> This method contains a redundant check of a known non-null value against the
> constant null.
> ProductPromoWorker.java:1556, NP_LOAD_OF_KNOWN_NULL_VALUE
> - NP: Load of known null value in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.performAction(ProductPromoWorker$ActionResultInfo,
> GenericValue, ShoppingCart, Delegator, LocalDispatcher, Timestamp)
> 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).
> ProductPromoWorker.java:1563, NP_LOAD_OF_KNOWN_NULL_VALUE
> - NP: Load of known null value in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.performAction(ProductPromoWorker$ActionResultInfo,
> GenericValue, ShoppingCart, Delegator, LocalDispatcher, Timestamp)
> 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).
> ProductPromoWorker.java:1951, UPM_UNCALLED_PRIVATE_METHOD
> UPM: Private method
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.findAdjustment(GenericValue,
> List) is never called
> This private method is never called. Although it is possible that the method
> will be invoked through reflection, it is more likely that the method is
> never used, and should be removed.
> ProductPromoWorker.java:2094, UC_USELESS_OBJECT
> Useless object created
> Our analysis shows that this object is useless. It's created and modified,
> but its value never go outside of the method or produce any side-effect.
> Either there is a mistake and object was intended to be used or it can be
> removed.
> This analysis rarely produces false-positives. Common false-positive cases
> include:
> - This object used to implicitly throw some obscure exception.
> - This object used as a stub to generalize the code.
> - This object used to hold strong references to weak/soft-referenced objects.
> ProductPromoWorker.java:2146, INT_BAD_COMPARISON_WITH_NONNEGATIVE_VALUE,
> Priorität: Normal
> INT: Bad comparison of nonnegative value with 0 in
> org.apache.ofbiz.order.shoppingcart.product.ProductPromoWorker.handleProductPromoCategories(Set,
> List, String, Delegator, Timestamp)
> This code compares a value that is guaranteed to be non-negative with a
> negative constant or zero.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
