Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Not A Problem
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.04
    • Component/s: accounting
    • Labels:
      None
    • Flags:
      Patch

      Description

      BillingAccountWorker.java:217, SE_NO_SERIALVERSIONID

      • SnVI: org.apache.ofbiz.accounting.payment.BillingAccountWorker$BillingAccountComparator 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.

      GiftCertificateServices.java:229, DLS_DEAD_LOCAL_STORE

      • DLS: Dead store to balance in org.apache.ofbiz.accounting.payment.GiftCertificateServices.addFundsToGiftCertificate(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.

      GiftCertificateServices.java:306, DLS_DEAD_LOCAL_STORE

      • DLS: Dead store to balance in org.apache.ofbiz.accounting.payment.GiftCertificateServices.redeemGiftCertificate(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.

      PaymentGatewayServices.java:211, UCF_USELESS_CONTROL_FLOW

      • UCF: Useless control flow in org.apache.ofbiz.accounting.payment.PaymentGatewayServices.authOrderPaymentPreference(DispatchContext, Map)
        This method contains a useless control flow statement, where control flow continues onto the same place regardless of whether or not the branch is taken.

      PaymentGatewayServices.java:1889, UCF_USELESS_CONTROL_FLOW

      • UCF: Useless control flow in org.apache.ofbiz.accounting.payment.PaymentGatewayServices.processAuthResult(DispatchContext, Map)
        This method contains a useless control flow statement, where control flow continues onto the same place regardless of whether or not the branch is taken.

      PaymentGatewayServices.java:3729, DLS_DEAD_LOCAL_STORE

      • DLS: Dead store to returnItemResponses in org.apache.ofbiz.accounting.payment.PaymentGatewayServices.isReplacementOrder(GenericValue)
        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.

        Activity

        Hide
        mbrohl Michael Brohl added a comment -

        > BTW there are also local variables which are declared public, though final. I saw a LOT of them in other services classes...

        Yes, I know, it's another big field of refactoring issues...

        Done in trunk r1804864, I did not back port because it's not a bug.

        Show
        mbrohl Michael Brohl added a comment - > BTW there are also local variables which are declared public, though final. I saw a LOT of them in other services classes... Yes, I know, it's another big field of refactoring issues... Done in trunk r1804864, I did not back port because it's not a bug.
        Hide
        jacques.le.roux Jacques Le Roux added a comment - - edited

        BTW there are also local variables which are declared public, though final. I saw a LOT of them in other services classes...

        Show
        jacques.le.roux Jacques Le Roux added a comment - - edited BTW there are also local variables which are declared public, though final. I saw a LOT of them in other services classes...
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi Michael,

        I had the same thought, so I agree, please do.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi Michael, I had the same thought, so I agree, please do.
        Hide
        mbrohl Michael Brohl added a comment -

        Hi Jacques,

        I agree with you about the initialization of balance because I assume that it should assure that the balance is returned as BigDecimal.ZERO in case of a failure or no data.

        I see no reason to declare a local variable ZERO from BigDecimal.ZERO. It's unneccessary overhead and since it it only used locally, it should also not be public.

        I suggest to remove the local variable ZERO and replace it with BigDecimal.ZERO.

        Show
        mbrohl Michael Brohl added a comment - Hi Jacques, I agree with you about the initialization of balance because I assume that it should assure that the balance is returned as BigDecimal.ZERO in case of a failure or no data. I see no reason to declare a local variable ZERO from BigDecimal.ZERO. It's unneccessary overhead and since it it only used locally, it should also not be public. I suggest to remove the local variable ZERO and replace it with BigDecimal.ZERO.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi Kira,

        I changed my mind. I see no reason why we would replace ZERO by null (no assignment) in both cases, the variables are used. So I revert r1804847 and I rather replace occurrences of BigDecimal.ZERO by the locally defined constant ZERO

        Done in
        trunk r1804859
        R16.11 r1804860

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi Kira, I changed my mind. I see no reason why we would replace ZERO by null (no assignment) in both cases, the variables are used. So I revert r1804847 and I rather replace occurrences of BigDecimal.ZERO by the locally defined constant ZERO Done in trunk r1804859 R16.11 r1804860
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Kyra,

        Your patch is in
        trunk r1804847
        R16.11 r1804848

        I did not backport more because it's not a biggie

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Kyra, Your patch is in trunk r1804847 R16.11 r1804848 I did not backport more because it's not a biggie
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi Kyra,

        Better to not use URLs for Jira issues in comments. Then Jira is able to show if the issue is closed (then striked out) or not, thanks

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi Kyra, Better to not use URLs for Jira issues in comments. Then Jira is able to show if the issue is closed (then striked out) or not, thanks
        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - - edited

        This patch only resolves bugs from GiftCertificateServices. The other bugs are either of no concern or dealt with in another ticket (OFBIZ-9530 -> Empty If-Statement in Method).

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - - edited This patch only resolves bugs from GiftCertificateServices. The other bugs are either of no concern or dealt with in another ticket ( OFBIZ-9530 -> Empty If-Statement in Method).

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            Kyra Pritzel-Hentley Kyra Pritzel-Hentley
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development