Details

      Description

      • MetricsFactory.java:236, ICAST_IDIV_CAST_TO_DOUBLE
        ICAST: Integral division result cast to double or float in org.apache.ofbiz.base.metrics.MetricsFactory$MetricsImpl.recordServiceRate(int, long)

      This code casts the result of an integral division (e.g., int or long division) operation to double or float. Doing division on integers truncates the result to the integer value closest to zero. The fact that the result was cast to double suggests that this precision should have been retained. What was probably meant was to cast one or both of the operands to double before performing the division. Here is an example:

      int x = 2;
      int y = 5;
      // Wrong: yields result 0.0
      double value1 = x / y;

      // Right: yields result 0.4
      double value2 = x / (double) y;

        Activity

        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Dennis,

        Because of possible errors, I consider this a fix and not an improvement.

        Your patch is in
        trunk r1804656
        R16.11 r1804657
        R15.12, R14.12, R13.07 r1804658

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Dennis, Because of possible errors, I consider this a fix and not an improvement. Your patch is in trunk r1804656 R16.11 r1804657 R15.12, R14.12, R13.07 r1804658
        Hide
        jacques.le.roux Jacques Le Roux added a comment - - edited

        Thanks Dennis,

        I see your point. I think we already had a discussion and a consensus on that because we follow Java Coding Conventions. So I'll not start a new discussion about that and will simply keep the parentheses.

        What makes things blurry to me is that APL's interpretation works like function composition. So in APL you would have to write it
        (10 * 2) + 50 * (1 - 0.5).
        Else; w/o 1st parentheses, you would get 270 instead of 45. And believe me after 20 years of such habit it's hard to read it another way, it's also very logical (function composition)

        Show
        jacques.le.roux Jacques Le Roux added a comment - - edited Thanks Dennis, I see your point. I think we already had a discussion and a consensus on that because we follow Java Coding Conventions . So I'll not start a new discussion about that and will simply keep the parentheses. What makes things blurry to me is that APL's interpretation works like function composition. So in APL you would have to write it (10 * 2) + 50 * (1 - 0.5) . Else; w/o 1st parentheses, you would get 270 instead of 45. And believe me after 20 years of such habit it's hard to read it another way, it's also very logical (function composition)
        Hide
        Dennis Balkir Dennis Balkir added a comment -

        Hi Jacques,
        I removed the parenthesis because I learned at Uni that they're logically not necessary. You can of course reimplement them, if you find it easier to read, I didn't want to change your style of programming, that's just the way I learned it and the way I'm doing it everytime

        Show
        Dennis Balkir Dennis Balkir added a comment - Hi Jacques, I removed the parenthesis because I learned at Uni that they're logically not necessary. You can of course reimplement them, if you find it easier to read, I didn't want to change your style of programming, that's just the way I learned it and the way I'm doing it everytime
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Hi Dennis,

        Why did you decide to remove parenthesis in serviceRate = (rate * smoothing) + (serviceRate * (1.0 - smoothing)); ?

        I find it easier to read like that. Disclaimer, I have been mostly using the APL language during my 1st 20 years of preogramming. In APL there is no notion of precedence, all expressions are evaluated from right to left (somehow like in Polish notation). So I have always find uneasy to read complicated algebric expressions in other languages

        Show
        jacques.le.roux Jacques Le Roux added a comment - Hi Dennis, Why did you decide to remove parenthesis in serviceRate = (rate * smoothing) + (serviceRate * (1.0 - smoothing)); ? I find it easier to read like that. Disclaimer, I have been mostly using the APL language during my 1st 20 years of preogramming. In APL there is no notion of precedence, all expressions are evaluated from right to left (somehow like in Polish notation). So I have always find uneasy to read complicated algebric expressions in other languages
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Ouch, this is the kind of stuff that you easily not see when reviewing, thanks FB

        Show
        jacques.le.roux Jacques Le Roux added a comment - Ouch, this is the kind of stuff that you easily not see when reviewing, thanks FB
        Hide
        Dennis Balkir Dennis Balkir added a comment -
        • changed a division with two long variables whose result was casted into a double; now it performs a proper double division
        Show
        Dennis Balkir Dennis Balkir added a comment - changed a division with two long variables whose result was casted into a double ; now it performs a proper double division

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            Dennis Balkir Dennis Balkir
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development