[jira] [Commented] (NUMBERS-149) port functions / tests in commons-lang for Fraction.
[ https://issues.apache.org/jira/browse/NUMBERS-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184198#comment-17184198 ] Jin Xu commented on NUMBERS-149: [~aherbert] got it. thanks for your detailed explanations. > port functions / tests in commons-lang for Fraction. > > > Key: NUMBERS-149 > URL: https://issues.apache.org/jira/browse/NUMBERS-149 > Project: Commons Numbers > Issue Type: Improvement > Components: fraction >Reporter: Jin Xu >Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > https://github.com/apache/commons-numbers/pull/82 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-149) port functions / tests in commons-lang for Fraction.
[
https://issues.apache.org/jira/browse/NUMBERS-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184189#comment-17184189
]
Alex Herbert commented on NUMBERS-149:
--
Adding the cache increase the memory footprint. I think this is a negative for
the primary use case of performing computations.
Note that hashCode is currently a branchless computation so a cache may not be
as performant as you expect. It is certainly an implementation detail. The
cache is implemented not using best practice anyway. The member variable should
be copied locally and this checked, computed if necessary and returned. This
avoids duplicate access to memory:
{code:java}
int value = this.value;
if (value == 0) {
value = this.value = computeValue();
}
return value;
{code}
Note: String is a special case where the hashCode computation can be expensive
if the string is long. String can also be interned and so common strings refer
to the same object and the cache is of more value.
The factory constructor using the whole and parts is currently served using:
{code:java}
int whole;
int numerator;
int denominator;
Fraction sum = Fraction.of(whole).add(Fraction.of(numerator, denominator));
{code}
Adding the method is not essential.
Porting the other methods marked as deprecated makes no sense. Anyone switching
APIs will be able to handle method name changes in 5 minutes.
The fix to Fraction.pow to handle MIN_VALUE should be in a separate ticket and
PR.
> port functions / tests in commons-lang for Fraction.
>
>
> Key: NUMBERS-149
> URL: https://issues.apache.org/jira/browse/NUMBERS-149
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Jin Xu
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> https://github.com/apache/commons-numbers/pull/82
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-149) port functions / tests in commons-lang for Fraction.
[ https://issues.apache.org/jira/browse/NUMBERS-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184183#comment-17184183 ] Jin Xu commented on NUMBERS-149: [~erans] btw about ValJO: If you think java.lang.String be ValJO, then actually String cached its hashcode. caching some things doesn't mean it is not ValJO IMO. > port functions / tests in commons-lang for Fraction. > > > Key: NUMBERS-149 > URL: https://issues.apache.org/jira/browse/NUMBERS-149 > Project: Commons Numbers > Issue Type: Improvement > Components: fraction >Reporter: Jin Xu >Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > https://github.com/apache/commons-numbers/pull/82 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-149) port functions / tests in commons-lang for Fraction.
[ https://issues.apache.org/jira/browse/NUMBERS-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184181#comment-17184181 ] Jin Xu commented on NUMBERS-149: [~erans] got it. will do the changes :) > port functions / tests in commons-lang for Fraction. > > > Key: NUMBERS-149 > URL: https://issues.apache.org/jira/browse/NUMBERS-149 > Project: Commons Numbers > Issue Type: Improvement > Components: fraction >Reporter: Jin Xu >Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > https://github.com/apache/commons-numbers/pull/82 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-149) port functions / tests in commons-lang for Fraction.
[
https://issues.apache.org/jira/browse/NUMBERS-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184174#comment-17184174
]
Gilles Sadowski commented on NUMBERS-149:
-
bq. Maybe I shouldn't add the @Deprecated tag?
The code is the problem, not the annotation!
bq. adding a cache, which I think good,
This change is not innocuous at could be at odds with the design goals:
{{Fraction}} is meant to be a
[ValJO|https://blog.joda.org/2014/03/valjos-value-java-objects.html]. Thus
requires discussion (on the "dev" ML).
This issue should only deal with ensuring that the concept of "fraction" is
correctly implemented in [Numbers], trimming all the cruft (bloat and bugs)
from the class in [Lang] but using it to double-check the implementation in
[Numbers].
> port functions / tests in commons-lang for Fraction.
>
>
> Key: NUMBERS-149
> URL: https://issues.apache.org/jira/browse/NUMBERS-149
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Jin Xu
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> https://github.com/apache/commons-numbers/pull/82
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Commented] (NUMBERS-149) port functions / tests in commons-lang for Fraction.
[ https://issues.apache.org/jira/browse/NUMBERS-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184156#comment-17184156 ] Jin Xu commented on NUMBERS-149: [~erans] Hi. I deprecated them to make people tend to use api in numbers directly Maybe I shouldn't add the @Deprecated tag? The toString() function is only changed for adding a cache, which I think good, and not change any format IMO. > port functions / tests in commons-lang for Fraction. > > > Key: NUMBERS-149 > URL: https://issues.apache.org/jira/browse/NUMBERS-149 > Project: Commons Numbers > Issue Type: Improvement > Components: fraction >Reporter: Jin Xu >Priority: Minor > Time Spent: 10m > Remaining Estimate: 0h > > https://github.com/apache/commons-numbers/pull/82 -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (NUMBERS-149) port functions / tests in commons-lang for Fraction.
[
https://issues.apache.org/jira/browse/NUMBERS-149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17184152#comment-17184152
]
Gilles Sadowski commented on NUMBERS-149:
-
Just browsing the top of the diffs...
This factory method
{code}
public static Fraction of(final int whole, final int numerator, final int
denominator)
{code}
is not necessary.
Neither are the many constants ({{ONE_HALF}}, {{ONE_THIRD}}, ...).
Why did you port code marked {{@Deprecated}}?
The {{toString()}} method should not be changed.
String formatting is not part of the intended API.
> port functions / tests in commons-lang for Fraction.
>
>
> Key: NUMBERS-149
> URL: https://issues.apache.org/jira/browse/NUMBERS-149
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
>Reporter: Jin Xu
>Priority: Minor
> Time Spent: 10m
> Remaining Estimate: 0h
>
> https://github.com/apache/commons-numbers/pull/82
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
