[ 
https://issues.apache.org/jira/browse/NUMBERS-129?focusedWorklogId=272636&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-272636
 ]

ASF GitHub Bot logged work on NUMBERS-129:
------------------------------------------

                Author: ASF GitHub Bot
            Created on: 05/Jul/19 15:04
            Start Date: 05/Jul/19 15:04
    Worklog Time Spent: 10m 
      Work Description: Schamschi commented on pull request #64: NUMBERS-129: 
Use long instead of int for intermediate results in Frac…
URL: https://github.com/apache/commons-numbers/pull/64
 
 
   …tion.addSub(Fraction, boolean)
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

            Worklog Id:     (was: 272636)
            Time Spent: 10m
    Remaining Estimate: 0h

> Waste of functionality in Fraction.addSub(Fraction, boolean)
> ------------------------------------------------------------
>
>                 Key: NUMBERS-129
>                 URL: https://issues.apache.org/jira/browse/NUMBERS-129
>             Project: Commons Numbers
>          Issue Type: Bug
>          Components: fraction
>    Affects Versions: 1.0
>            Reporter: Heinrich Bohne
>            Priority: Minor
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> [NUMBERS-79|https://issues.apache.org/jira/browse/NUMBERS-79], which has been 
> marked as resolved by now, suggested to replace the usage of {{BigInteger}} 
> in the method {{Fraction.addSub(Fraction, boolean)}} with primitive {{long}} 
> values for the sake of performance. However, the ticket's 
> "[resolution|https://github.com/apache/commons-numbers/commit/7c2486362b64201fd8dc19b2df12aefba2a4165a]";
>  does not use {{long}}, but {{int}} variables (as I hinted at in a comment in 
> the ticket about a month ago, when the code was still in the branch 
> fraction-dev and not merged into master yet), although the method's 
> documentation was changed to state that the method uses the {{long}} datatype.
> So for one thing, the method's documentation makes claims about the method's 
> implementation that are simply not true, and for another, the usage of 
> {{int}}s can cause the method to fail, while using {{long}}s would _always_ 
> produce the correct result because an overflow would never occur (which I 
> also explained in the aforementioned comment in the ticket).
> As for performance: I doubt that three {{int}}\-to\-{{long}} conversions 
> and subsequent multiplications and one {{Math.toIntExact(long)}} invocation 
> are significantly less performant than three {{Math.multiplyExact(int, int)}} 
> and one {{Math.addExact(int, int)}} invocation (if at all), so using 
> {{int}}s rather than {{long}}s sacrifices functionality for practically 
> nothing.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to