[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16876445#comment-16876445
]
Heinrich Bohne commented on NUMBERS-123:
----------------------------------------
While pondering over the impact of converting this constructor to a factory
method (and over the possible reasons for choosing one over the other in
general), I couldn't help but notice one consequence that I would consider a
drawback:
The constructor {{BigFraction(BigInteger, BigInteger)}} not only initializes
the fields {{numerator}} and {{denominator}}, but it also ascertains that the
denominator is not zero, it ensures that the sign, if present, is held by the
numerator, and it reduces the numerator and denominator to lowest terms.
All of these conditions are already fulfilled by the values passed to this
constructor from the method {{from(double)}}, even if they were enforced in a
completely different way, so subjecting the calculated numerator and
denominator to the checks and conversions of the constructor
{{BigFraction(BigInteger, BigInteger)}} is redundant. I don't think this
redundancy is outweighed by the benefits of having the code inside a static
factory method instead of a constructor (although admittedly, this is because I
don't really know what these benefits are), so actually, I find the original
design where this code was inside a constructor to be preferable (by the way,
the same also applies to the constructor converted in
[NUMBERS-124|https://issues.apache.org/jira/browse/NUMBERS-124]). What do you
think, Gilles?
> "BigFraction(double)" is unnecessary
> ------------------------------------
>
> Key: NUMBERS-123
> URL: https://issues.apache.org/jira/browse/NUMBERS-123
> Project: Commons Numbers
> Issue Type: Improvement
> Components: fraction
> Reporter: Gilles
> Assignee: Gilles
> Priority: Trivial
> Fix For: 1.0
>
> Attachments: NUMBERS-123__Javadoc.patch
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Constructor {{BigFraction(double value)}} is only called from the
> {{from(double value)}} method.
> Actually, this constructor is misleading as it is indeed primarily a
> conversion from which appropriate {{numerator}} and {{denominator}} fields
> are computed; those could be set by
> the "direct" constructor {{BigFraction(BigInteger num, BigInteger den)}}.
> Moreover, the private field {{ZERO}} goes through this conversion code
> whereas it could constructed "directly", e.g. using {{of(0)}}. Similarly for
> field {{ONE}}.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
