Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Implemented
    • Affects Version/s: Trunk
    • Fix Version/s: Upcoming Release
    • Component/s: workeffort
    • Labels:
      None

      Description

      ICalConverter.java:229, DM_FP_NUMBER_CTOR

      • Bx: org.apache.ofbiz.workeffort.workeffort.ICalConverter.fromDuration(PropertyList) invokes inefficient new Double(double) constructor; use Double.valueOf(double) instead

      Using new Double(double) is guaranteed to always result in a new object whereas Double.valueOf(double) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.

      Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Double and Float.

      ICalConverter.java:261, DM_NUMBER_CTOR

      • Bx: org.apache.ofbiz.workeffort.workeffort.ICalConverter.fromPercentComplete(PropertyList) invokes inefficient new Long(long) constructor; use Long.valueOf(long) instead

      Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.

      Values between -128 and 127 are guaranteed to have corresponding cached instances and using valueOf is approximately 3.5 times faster than using constructor. For values outside the constant range the performance of both styles is the same.

      Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Long, Integer, Short, Character, and Byte.

      ICalConverter.java:269, DM_FP_NUMBER_CTOR

      • Bx: org.apache.ofbiz.workeffort.workeffort.ICalConverter.fromPriority(PropertyList) invokes inefficient new Double(double) constructor; use Double.valueOf(double) instead

      Using new Double(double) is guaranteed to always result in a new object whereas Double.valueOf(double) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.

      Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Double and Float.

      ICalConverter.java:479, REC_CATCH_EXCEPTION

      • REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.workeffort.workeffort.ICalConverter.invokeService(String, Map, 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 ... }

      ICalConverter.java:506, RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE
      - RCN: Nullcheck of partyAssign at line 516 of value previously dereferenced in org.apache.ofbiz.workeffort.workeffort.ICalConverter.loadPartyAssignment(Property, GenericValue, Map)

      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.

      ICalConverter.java:789, DM_CONVERT_CASE
      - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in org.apache.ofbiz.workeffort.workeffort.ICalConverter.storePartyAssignments(String, Component, Map)

      A String is being converted to upper or lowercase, using the platform's default encoding. This may result in improper conversions when used with international characters. Use the

      String.toUpperCase( Locale l )
      String.toLowerCase( Locale l )
      versions instead.

      ICalRecurConverter.java:45, MS_PKGPROTECT
      - MS: org.apache.ofbiz.workeffort.workeffort.ICalRecurConverter.dayOfWeekArray should be package protected

      A mutable static field could be changed by malicious code or by accident. The field could be made package protected to avoid this vulnerability.

      ICalRecurConverter.java:251, SF_SWITCH_NO_DEFAULT
      - SF: Switch statement found in org.apache.ofbiz.workeffort.workeffort.ICalRecurConverter.visit(TemporalExpressions$Frequency) where default case is missing

      This method contains a switch statement where default case is missing. Usually you need to provide a default case.

      Because the analysis only looks at the generated bytecode, this warning can be incorrect triggered if the default case is at the end of the switch statement and the switch statement doesn't contain break statements for other cases.

      ICalWorker.java:216, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.workeffort.workeffort.ICalWorker.handlePropFindRequest(HttpServletRequest, HttpServletResponse, ServletContext)

      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 ... }

      WorkEffortKeywordIndex.java:48, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
      - RCN: Redundant nullcheck of delegator, which is known to be non-null in org.apache.ofbiz.workeffort.workeffort.WorkEffortKeywordIndex.indexKeywords(GenericValue)

      This method contains a redundant check of a known non-null value against the constant null.

      WorkEffortKeywordIndex.java:64, DM_CONVERT_CASE
      - Dm: Use of non-localized String.toUpperCase() or String.toLowerCase() in org.apache.ofbiz.workeffort.workeffort.WorkEffortKeywordIndex.indexKeywords(GenericValue)

      A String is being converted to upper or lowercase, using the platform's default encoding. This may result in improper conversions when used with international characters. Use the

      String.toUpperCase( Locale l )
      String.toLowerCase( Locale l )
      versions instead.

      WorkEffortSearch.java:474, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$WorkEffortAssocConstraint is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearch.java:587, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS
      - BC: Equals method for org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$WorkEffortAssocConstraint assumes the argument is of type WorkEffortSearch$WorkEffortAssocConstraint

      The equals(Object o) method shouldn't make any assumptions about the type of o. It should simply return false if o is not the same type as this.

      WorkEffortSearch.java:587, HE_EQUALS_USE_HASHCODE
      - HE: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$WorkEffortAssocConstraint 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 }

      WorkEffortSearch.java:623, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$WorkEffortReviewConstraint is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearch.java:653, HE_EQUALS_USE_HASHCODE
      - HE: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$WorkEffortReviewConstraint 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 }

      WorkEffortSearch.java:653, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS
      - BC: Equals method for org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$WorkEffortReviewConstraint assumes the argument is of type WorkEffortSearch$WorkEffortReviewConstraint

      The equals(Object o) method shouldn't make any assumptions about the type of o. It should simply return false if o is not the same type as this.

      WorkEffortSearch.java:678, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$PartyAssignmentConstraint is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearch.java:755, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS
      - BC: Equals method for org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$PartyAssignmentConstraint assumes the argument is of type WorkEffortSearch$PartyAssignmentConstraint

      The equals(Object o) method shouldn't make any assumptions about the type of o. It should simply return false if o is not the same type as this.

      WorkEffortSearch.java:755, HE_EQUALS_USE_HASHCODE
      - HE: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$PartyAssignmentConstraint 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 }

      WorkEffortSearch.java:788, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$ProductSetConstraint is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearch.java:852, HE_EQUALS_USE_HASHCODE
      - HE: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$ProductSetConstraint 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 }

      WorkEffortSearch.java:852, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS
      - BC: Equals method for org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$ProductSetConstraint assumes the argument is of type WorkEffortSearch$ProductSetConstraint

      The equals(Object o) method shouldn't make any assumptions about the type of o. It should simply return false if o is not the same type as this.

      WorkEffortSearch.java:880, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$KeywordConstraint is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearch.java:961, HE_EQUALS_USE_HASHCODE
      - HE: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$KeywordConstraint 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 }

      WorkEffortSearch.java:961, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS
      - BC: Equals method for org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$KeywordConstraint assumes the argument is of type WorkEffortSearch$KeywordConstraint

      The equals(Object o) method shouldn't make any assumptions about the type of o. It should simply return false if o is not the same type as this.

      WorkEffortSearch.java:998, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$LastUpdatedRangeConstraint is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearch.java:999, EI_EXPOSE_REP2
      - EI2: new org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$LastUpdatedRangeConstraint(Timestamp, Timestamp) may expose internal representation by storing an externally mutable object into WorkEffortSearch$LastUpdatedRangeConstraint.fromDate

      This code stores a reference to an externally mutable object into the internal representation of the object. If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.

      WorkEffortSearch.java:1000, EI_EXPOSE_REP2
      - EI2: new org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$LastUpdatedRangeConstraint(Timestamp, Timestamp) may expose internal representation by storing an externally mutable object into WorkEffortSearch$LastUpdatedRangeConstraint.thruDate

      This code stores a reference to an externally mutable object into the internal representation of the object. If instances are accessed by untrusted code, and unchecked changes to the mutable object would compromise security or other important properties, you will need to do something different. Storing a copy of the object is better approach in many situations.

      WorkEffortSearch.java:1049, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTSl
      - BC: Equals method for org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$LastUpdatedRangeConstraint assumes the argument is of type WorkEffortSearch$LastUpdatedRangeConstraint

      The equals(Object o) method shouldn't make any assumptions about the type of o. It should simply return false if o is not the same type as this.

      WorkEffortSearch.java:1049, HE_EQUALS_USE_HASHCODE
      - HE: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$LastUpdatedRangeConstraint 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 }

      WorkEffortSearch.java:1094, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$SortKeywordRelevancy is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearch.java:1134, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearch$SortWorkEffortField is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearchSession.java:46, SE_NO_SERIALVERSIONID
      - SnVI: org.apache.ofbiz.workeffort.workeffort.WorkEffortSearchSession$WorkEffortSearchOptions is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      WorkEffortSearchSession.java:308, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
      - RCN: Redundant nullcheck of resultSortOrder, which is known to be non-null in org.apache.ofbiz.workeffort.workeffort.WorkEffortSearchSession.searchGetSortOrderString(boolean, HttpServletRequest)

      This method contains a redundant check of a known non-null value against the constant null.

      WorkEffortServices.java:509, DLS_DEAD_LOCAL_STORE
      - DLS: Dead store to filterOutCanceledEvents in org.apache.ofbiz.workeffort.workeffort.WorkEffortServices.getWorkEffortEventsByPeriod(DispatchContext, Map)

      This instruction assigns a value to a local variable, but the value is not read or used in any subsequent instruction. Often, this indicates an error, because the value computed is never used.

      Note that Sun's javac compiler often generates dead stores for final local variables. Because FindBugs is a bytecode-based tool, there is no easy way to eliminate these false positives.

      WorkEffortServices.java:1075, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.workeffort.workeffort.WorkEffortServices.removeDuplicateWorkEfforts(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 ... }

        Activity

        Hide
        jleichert Julian Leichert added a comment -

        class ICalConverter

        • Fixed minor Findbugs warnings.
        • removed redundant null-check at line 516

        class ICalRecurConverter

        • added default to switch statement

        class ICalWorker

        • added Multicatch

        class WorkEffortKeywordIndex

        • added charset

        class WorkEffortSearch

        • added hash codes
        • adapted equal-methods

        class WorkEffortSearchSession

        • removed redundant null-check
        • removed trailing whitespaces

        class WorkEffortServices

        • added Diamond Operator
        • changed catch to GenericEntityException
        Show
        jleichert Julian Leichert added a comment - class ICalConverter Fixed minor Findbugs warnings. removed redundant null-check at line 516 class ICalRecurConverter added default to switch statement class ICalWorker added Multicatch class WorkEffortKeywordIndex added charset class WorkEffortSearch added hash codes adapted equal-methods class WorkEffortSearchSession removed redundant null-check removed trailing whitespaces class WorkEffortServices added Diamond Operator changed catch to GenericEntityException
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Julian,

        your patch is in trunk r1812900.

        Show
        mbrohl Michael Brohl added a comment - Thanks Julian, your patch is in trunk r1812900.

          People

          • Assignee:
            mbrohl Michael Brohl
            Reporter:
            jleichert Julian Leichert
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development