[
https://issues.apache.org/jira/browse/NUMBERS-123?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16876910#comment-16876910
]
Heinrich Bohne commented on NUMBERS-123:
----------------------------------------
bq. The sole purpose of the flag is efficiency
Actually, I wasn't concerned about efficiency at all when I raised this issue.
I found the design itself a bit awkward, because I think code should not do
more than it needs to do; efficiency is only a by-product of this. And because
I think that it is indeed the responsibility of the constructor (whether it is
private or public) to validate the values to which it initializes the fields, I
don't think the flag-approach is a satisfactory solution (of course, one could
include a warning in the documentation of the constructor that it must only be
called with valid arguments, but then, I really don't see how this would be
better than having three separate constructors).
bq. this discussion also is a hint that the initial setup was not clear
Indeed, it was not obvious that the three constructors all reduced the fraction
to lowest terms. A comment would probably have been in order. Although I don't
see what this has to do with the awkward initialization of the ZERO and ONE
constants. As far as I can tell, this is simply a matter of which
method/constructor to call – converting the constructors to factory methods
should make a difference there.
In the end, I think it boils down to the question of whether one is willing to
entrust the code that converts a double value to a fraction with the
responsibility of ensuring that the calculated numerator and denominator
fulfill the conditions required by the fields' contracts (i.e. lowest terms,
sign in numerator etc.), which is not unreasonable, since the two double
constructors/methods accomplish this without code duplication. If yes, then the
code would be better of in constructors, like in the original design. If not,
then the current design seems the most adequate.
> "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)
