Details

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

      Description

      • UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
        CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Addition is final but declares protected field org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Addition.newValue

      This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.

      • UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
        CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Removal is final but declares protected field org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Removal.oldValue

      This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.

      • UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
        CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update is final but declares protected field org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update.newValue

      This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.

      • UtilCacheTests.java:-1, CI_CONFUSED_INHERITANCE
        CI: Class org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update is final but declares protected field org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Update.oldValue

      This class is declared to be final, but declares fields to be protected. Since the class is final, it can not be derived from, and the use of protected is confusing. The access modifier for the field should be changed to private or public to represent the true use for the field.

      • UtilCacheTests.java:39, SE_NO_SUITABLE_CONSTRUCTOR
        Se: org.apache.ofbiz.base.util.cache.test.UtilCacheTests is Serializable but its superclass doesn't define an accessible void constructor

      This class implements the Serializable interface and its superclass does not. When such an object is deserialized, the fields of the superclass need to be initialized by invoking the void constructor of the superclass. Since the superclass does not have one, serialization and deserialization will fail at runtime.

      • UtilCacheTests.java:39, SE_NO_SERIALVERSIONID
        SnVI: org.apache.ofbiz.base.util.cache.test.UtilCacheTests 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.

      • UtilCacheTests.java:148, HE_EQUALS_USE_HASHCODE
        HE: org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Listener 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 }
      • UtilCacheTests.java:148, NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT
        NP: org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Listener.equals(Object) does not check for null argument

      This implementation of equals(Object) violates the contract defined by java.lang.Object.equals() because it does not check for null being passed as the argument. All equals() methods should return false if passed a null value.

      • UtilCacheTests.java:148, BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS
        BC: Equals method for org.apache.ofbiz.base.util.cache.test.UtilCacheTests$Listener assumes the argument is of type UtilCacheTests$Listener

      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.

      • UtilCacheTests.java:345, DM_STRING_CTOR
        Dm: org.apache.ofbiz.base.util.cache.test.UtilCacheTests.testPutIfAbsentAndGet() invokes inefficient new String(String) constructor

      Using the java.lang.String(String) constructor wastes memory because the object so constructed will be functionally indistinguishable from the String passed as a parameter. Just use the argument String directly.

      • UtilCacheTests.java:350, ES_COMPARING_STRINGS_WITH_EQ
        ES: Comparison of String objects using == or != in org.apache.ofbiz.base.util.cache.test.UtilCacheTests.testPutIfAbsentAndGet()

      This code compares java.lang.String objects for reference equality using the == or != operators. Unless both strings are either constants in a source file, or have been interned using the String.intern() method, the same string value may be represented by two different String objects. Consider using the equals(Object) method instead.

      • UtilCacheTests.java:365, DM_STRING_CTOR
        Dm: org.apache.ofbiz.base.util.cache.test.UtilCacheTests.testChangeMemSize() invokes inefficient new String(String) constructor

      Using the java.lang.String(String) constructor wastes memory because the object so constructed will be functionally indistinguishable from the String passed as a parameter. Just use the argument String directly.

      • UtilCacheTests.java:396, DM_STRING_CTOR
        Dm: org.apache.ofbiz.base.util.cache.test.UtilCacheTests.expireTest(UtilCache, int, long) invokes inefficient new String(String) constructor

      Using the java.lang.String(String) constructor wastes memory because the object so constructed will be functionally indistinguishable from the String passed as a parameter. Just use the argument String directly.

        Activity

        Hide
        Dennis Balkir Dennis Balkir added a comment -
        • fixed Diamond Operators
        • Line 39: didn't fix error, serialVersionUID doesn't have to be implemented for this test class
        • class Removal: changed oldValue from protected to private
        • class Addition: changed newValue from protected to private
        • class Update: changed newValue and oldValue from protected to private
        • added type-check in equals to prevent casting errors
        • implemented hashCode(), because equals was implemented (it still throws a findbugs bug, because it just uses the super.hashCode()
        • Line 353: didn't change anything, if the declaration is changed, it won't work
        • Line 354: didn't change anything, if the declaration is changed, it won't work
        • put code, which was used more often in an own method called assertKeyLoop, because it is DRY, but didn't change the String declaration, because it was used to clone the String
        • Line 358: didn't fix anything, method was planned this way
        Show
        Dennis Balkir Dennis Balkir added a comment - fixed Diamond Operators Line 39: didn't fix error, serialVersionUID doesn't have to be implemented for this test class class Removal: changed oldValue from protected to private class Addition: changed newValue from protected to private class Update: changed newValue and oldValue from protected to private added type-check in equals to prevent casting errors implemented hashCode() , because equals was implemented (it still throws a findbugs bug, because it just uses the super.hashCode() Line 353: didn't change anything, if the declaration is changed, it won't work Line 354: didn't change anything, if the declaration is changed, it won't work put code, which was used more often in an own method called assertKeyLoop , because it is DRY, but didn't change the String declaration, because it was used to clone the String Line 358: didn't fix anything, method was planned this way
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Dennis,

        your patch is in trunk r1811416.

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

          People

          • Assignee:
            mbrohl Michael Brohl
            Reporter:
            Dennis Balkir Dennis Balkir
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development