Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: Upcoming Release
    • Component/s: accounting
    • Labels:
      None
    • Flags:
      Patch

      Description

      CCPaymentServices.java:78: 109: 144: 178: 235: 270, DM_BOOLEAN_CTOR

      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.ccAuth(DispatchContext, Map) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.ccCredit(DispatchContext, Map) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.ccCapture(DispatchContext, Map) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.ccRelease(DispatchContext, Map) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.ccRefund(DispatchContext, Map) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.ccReAuth(DispatchContext, Map) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
        Creating new instances of java.lang.Boolean wastes memory, since Boolean objects are immutable and there are only two useful values of this type. Use the Boolean.valueOf() method (or Java 1.5 autoboxing) to create Boolean objects instead.

      CCPaymentServices.java:772, SBSC_USE_STRINGBUFFER_CONCATENATION

      • SBSC: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.appendPaymentMechNode(Element, GenericValue, String, String) concatenates strings using + in a loop
        The method seems to be building a String using concatenation in a loop. In each iteration, the String is converted to a StringBuffer/StringBuilder, appended to, and converted back to a String. This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.
        Better performance can be obtained by using a StringBuffer (or StringBuilder in Java 1.5) explicitly.

      CCPaymentServices.java:902, DM_DEFAULT_ENCODING

      • Dm: Found reliance on default encoding in org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCPaymentServices.sendRequest(Document, String, Delegator): java.io.ByteArrayOutputStream.toString()
        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 behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.

      CCServicesTest.java:101: 131: 162: 209, DM_BOOLEAN_CTOR, Priorität: Normal

      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCServicesTest.testAuth() invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCServicesTest.testCredit() invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCServicesTest.testPurchaseSubscription() invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
      • Dm: org.apache.ofbiz.accounting.thirdparty.clearcommerce.CCServicesTest.testCCReport() invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead
        Creating new instances of java.lang.Boolean wastes memory, since Boolean objects are immutable and there are only two useful values of this type. Use the Boolean.valueOf() method (or Java 1.5 autoboxing) to create Boolean objects instead.

        Activity

        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment -

        The bug DM_DEFAULT_ENCODING does not need a fix since the method UtilXml.writeXmlDocument(requestDocument, os, "UTF-8", true, false, 0) is called before converting the stream to a string. The method writes xml documents which are always encoded in UTF-8 and have an output file which corresponds to the encoding given as an argument (which is UTF-8 as well). Therefore, there should be no varying behavior between platforms.

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - The bug DM_DEFAULT_ENCODING does not need a fix since the method UtilXml.writeXmlDocument(requestDocument, os, "UTF-8", true, false, 0) is called before converting the stream to a string. The method writes xml documents which are always encoded in UTF-8 and have an output file which corresponds to the encoding given as an argument (which is UTF-8 as well). Therefore, there should be no varying behavior between platforms.
        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment -

        The patch resolves all bugs except for DM_DEFAULT_ENCODING.

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - The patch resolves all bugs except for DM_DEFAULT_ENCODING.
        Hide
        mbrohl Michael Brohl added a comment -

        Hi Kyra,

        thanks for your patch!

        I suggest to substitute the pattern

        Boolean.valueOf(false)
        

        with

        Boolean.FALSE
        

        in your patch to be more efficient.

        Show
        mbrohl Michael Brohl added a comment - Hi Kyra, thanks for your patch! I suggest to substitute the pattern Boolean .valueOf( false ) with Boolean .FALSE in your patch to be more efficient.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi Michael,

        What does the [FB] prefix in title means? Thanks.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi Michael, What does the [FB] prefix in title means? Thanks.
        Hide
        mbrohl Michael Brohl added a comment -

        It stands for FindBugs. It should make clear which code analysis tool is the base of the found bugs and corrections.
        If people check the code with PMD, they can apply [PMD].

        Show
        mbrohl Michael Brohl added a comment - It stands for FindBugs. It should make clear which code analysis tool is the base of the found bugs and corrections. If people check the code with PMD, they can apply [PMD] .
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        OK, thanks

        Show
        jacques.le.roux Jacques Le Roux added a comment - OK, thanks
        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment -

        Hi Michael,

        thank you for your suggestion! I have updated the patch, replacing Boolean.valueOf(false) with Boolean.FALSE.

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - Hi Michael, thank you for your suggestion! I have updated the patch, replacing Boolean.valueOf(false) with Boolean.FALSE.
        Hide
        tlaufkoetter Tobias Laufkötter added a comment -

        Instead of padding the cardSecurityCode in a loop you can use a formatter
        String.format("%-4s", cardSecurityCode); will do a right padding with the appropriate amount of spaces.

        Show
        tlaufkoetter Tobias Laufkötter added a comment - Instead of padding the cardSecurityCode in a loop you can use a formatter String.format("%-4s", cardSecurityCode); will do a right padding with the appropriate amount of spaces.
        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment -

        The String formatter works great! Thank you Tobias for your suggestion.
        Additionally the new patch changes

         if (((Boolean) result.get("authResult")).equals(new Boolean(false))) 

        to

         if ((Boolean.FALSE).equals((Boolean) result.get("authResult"))) 

        in order to prevent Null Pointer Exceptions.

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - The String formatter works great! Thank you Tobias for your suggestion. Additionally the new patch changes if ((( Boolean ) result.get( "authResult" )).equals( new Boolean ( false ))) to if (( Boolean .FALSE).equals(( Boolean ) result.get( "authResult" ))) in order to prevent Null Pointer Exceptions.
        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment -

        This patch should finally cover all above mentioned bugs. The class CCServicesTest needed another assertion of the of authResult parameter to check if it is null and then fail the test. (Since no more NullPointerException is thrown with the equals() method)

        While trying to test the class I learned that CCServicesTest is actually never tested (not included in any test-suite). I wonder why this class is still in the OFBiz project if it is never used for testing. And after adding the class to a suite, the class failed 4 out of 6 tests. This, of course, is a topic for another ticket but is probably worth looking into.

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - This patch should finally cover all above mentioned bugs. The class CCServicesTest needed another assertion of the of authResult parameter to check if it is null and then fail the test. (Since no more NullPointerException is thrown with the equals() method) While trying to test the class I learned that CCServicesTest is actually never tested (not included in any test-suite). I wonder why this class is still in the OFBiz project if it is never used for testing. And after adding the class to a suite, the class failed 4 out of 6 tests. This, of course, is a topic for another ticket but is probably worth looking into.
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Kyra,

        your patch is in trunk r1811407.

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

          People

          • Assignee:
            mbrohl Michael Brohl
            Reporter:
            Kyra Pritzel-Hentley Kyra Pritzel-Hentley
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development