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

      The following Bugs were found by the FindBugs Software in the OFBiz codebase and may need fixing:

      GeneralLedgerServices.java:42, DLS_DEAD_LOCAL_STORE

      • DLS: Dead store to totalAmountPercentage in org.apache.ofbiz.accounting.ledger.GeneralLedgerServices.createUpdateCostCenter(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.

      GeneralLedgerServices.java:50, WMI_WRONG_MAP_ITERATOR

      • WMI: org.apache.ofbiz.accounting.ledger.GeneralLedgerServices.createUpdateCostCenter(DispatchContext, 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.

      GeneralLedgerServices.java:73, WMI_WRONG_MAP_ITERATOR

      • WMI: org.apache.ofbiz.accounting.ledger.GeneralLedgerServices.calculateCostCenterTotal(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.

      GeneralLedgerServices.java:75, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

      • RCN: Redundant nullcheck of rowValue, which is known to be non-null in org.apache.ofbiz.accounting.ledger.GeneralLedgerServices.calculateCostCenterTotal(Map)
        This method contains a redundant check of a known non-null value against the constant null.

        Activity

        Hide
        pandeypranay Pranay Pandey added a comment -

        Hello Kyra Pritzel-Hentley,

        Thanks for reporting the issue. Please put a clear summary of the issue in ticket summary and provide steps to regenerate the issue in the description. Both of these details are very important in tracking down the issue. It's also required even if you are going to provide a patch for fixing the reported issue.

        It's recommended study for every OFBiz contributor - https://cwiki.apache.org/confluence/display/OFBIZ/OFBiz+Contributors+Best+Practices#OFBizContributorsBestPractices-HowtoContributetoOFBiz

        Thanks!

        Show
        pandeypranay Pranay Pandey added a comment - Hello Kyra Pritzel-Hentley, Thanks for reporting the issue. Please put a clear summary of the issue in ticket summary and provide steps to regenerate the issue in the description. Both of these details are very important in tracking down the issue. It's also required even if you are going to provide a patch for fixing the reported issue. It's recommended study for every OFBiz contributor - https://cwiki.apache.org/confluence/display/OFBIZ/OFBiz+Contributors+Best+Practices#OFBizContributorsBestPractices-HowtoContributetoOFBiz Thanks!
        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment -

        Hello Pranay Pandey,

        Thank you for the hints. I haven't written that much of a summary since this ticket describes bugs found by a code analysis tool. The parent ticket gives a further description of what is being done here. I don't quite know how I would go about describing how to generate the issue since I am only looking at the code. Do you have any suggestions for this?

        I will read the best practices guide. Thank you!

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - Hello Pranay Pandey, Thank you for the hints. I haven't written that much of a summary since this ticket describes bugs found by a code analysis tool. The parent ticket gives a further description of what is being done here. I don't quite know how I would go about describing how to generate the issue since I am only looking at the code. Do you have any suggestions for this? I will read the best practices guide. Thank you!
        Hide
        pandeypranay Pranay Pandey added a comment -

        Thanks for the comment Kyra. Sorry my bad, I didn't realise that. I haven't gone through details on code analysis tool yet.

        Show
        pandeypranay Pranay Pandey added a comment - Thanks for the comment Kyra. Sorry my bad, I didn't realise that. I haven't gone through details on code analysis tool yet.
        Hide
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment -

        The patch resolves the found bugs.
        It made sense to resolve the WMI_WRONG_MAP_ITERATOR bug since the method was iterating over the whole map twice.

        Show
        Kyra Pritzel-Hentley Kyra Pritzel-Hentley added a comment - The patch resolves the found bugs. It made sense to resolve the WMI_WRONG_MAP_ITERATOR bug since the method was iterating over the whole map twice.
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Kyra,

        your modified patch is in trunk r1805464.

        Additionally to the provided patch I removed the static ZERO variable
        and changed the assignment to BigDecimal.ZERO instead. An unused import
        statement was removed also.

        Show
        mbrohl Michael Brohl added a comment - Thanks Kyra, your modified patch is in trunk r1805464. Additionally to the provided patch I removed the static ZERO variable and changed the assignment to BigDecimal.ZERO instead. An unused import statement was removed also.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development