As the author of the blog and term VALJO, here are some comments on Fraction:
You should use `of()` (overloading allowed) when the factory normally succeeds.
You should use `from` (overloading allowed) when the factory methods
are performing a conversion and have a reasonable chance of failure.
The `int` methods should use `of`. The `double` methods could use
either, it is a judgement call as top whether it is a conversion or a
construction (does it normally succeed or not).
Looking at this code
https://git-wip-us.apache.org/repos/asf?p=commons-numbers.git;a=blob;f=commons-numbers-fraction/src/main/java/org/apache/commons/numbers/fraction/Fraction.java;h=308f93033853ca8815663c576f7c38e6770dc3c3;hb=HEAD
In the `abs()` method, there is no need for a local variable - just
return from each branch of the if statement or use a ternary.
The method order in the class is strange. I would recommend putting
the getters first. I would also recommend grouping the methods
compareTo, equals, hashCode and toString in that order at the end of
the class. See `LocalDate` for example.
The order of the static constants is also strange - I'm sure a more
logical order could be chosen.
The method `getReducedFraction` is not a getter, so it should not
start with `get`. Maybe `ofReduced()` ? Alternatively, have an
instance method `reduced()` that can be called on any fraction, so
users do `of(2, 4).reduce()`.
The recommended naming approach for methods on immutable VALJO classes
is to use the past tense:
multiply -> multipliedBy
divide -> dividedBy
add -> plus
subtract -> minus
negate -> negated
No doubt this would apply widely in the project
HTH
Stephen
Post by GillesPost by Eric BarnhillOh right, that is the convention. I knew there was something off.
As far as you understand, is to within VALJO standards to overload factory
methods,
I don't think that it is ValJO-related; method overload is a
feature, so better use it rather than duplicate what the compiler
can do by itself. ;-)
Gilles
Post by Eric Barnhillso long as they are not private constructors? All that is
specified on the page is that VALJOs must have all constructors private. So
I am not sure whether it is in the spirit of VALJOs to overload, but coming
up with elaborate names for each constructor doesn't seem like a very
streamlined coding practice.
Post by Eric BarnhillPost by Eric BarnhillThe Fraction class is IMO looking good (in better shape than
Complex
Post by Eric Barnhillwas
in) and is already quite close to fulfilling the standards for a VALJO.
Equals() and CompareTo() are well designed and consistent. I see
two
Post by Eric Barnhillmissing steps. The easy one is a parse() method which mirrors the
toString() method. The harder one is the wide range of public constructors.
To be a VALJO all constructors must be private and accessed with static
factory methods. If these factory methods themselves can be overloaded, I
current constructor -> proposed factory method
--------------------------------------------------------
public Fraction(double value) -> public fromDouble(double value)
public Fraction(double value, double epsilon, int maxIterations)
->
Post by Eric Barnhillpublic
fromDouble(double value, double epsilon, int maxIterations)
public Fraction(double value,int maxDenominator) -> public fromDouble
(double value,int maxDenominator)
public Fraction(int value) -> public fromInt(int value)
public Fraction(int num, int denom) -> public fromInt(int num, int denom)
Why not call them all "of(...)" ?
Gilles
Post by Eric Barnhillso this is what I propose to go with.
If disambiguation in the double cases is still a problem, the
second
Post by Eric Barnhilland
third of the double constructors could be fromDoubleEpsMaxInt and
fromDoubleMaxDenom .
Eric
On Thu, Oct 11, 2018 at 7:00 AM Gilles
Post by GillesPost by Eric BarnhillI am interested in moving forward on making the Fraction
classes
Post by Eric BarnhillPost by GillesPost by Eric BarnhillVALJOs
[NUMBERS-75].
Just a preliminary question for now, are we otherwise happy
with
Post by Eric BarnhillPost by Gillesthe
Post by Eric BarnhillFraction class in the context of commons-numbers? Or should I
look
Post by Eric BarnhillPost by GillesPost by Eric Barnhillaround
for any odd behaviors leftover from commons-math (Complex had a
lot
Post by Eric Barnhillof
those) that might also be improved?
AFAIK, there was no in-depth review as was done for "Complex".
So it would indeed be quite useful to check what the Javadoc
states, whether it seems acceptable (wrt what other libraries
do), and whether the unit tests validate everything.
Side note: Unless I'm overlooking something, completing this
task will result in getting rid of all the formatting and
"Locale"-related classes (as for "Complex").
Best,
Gilles
---------------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-***@commons.apache.org
For additional commands, e-mail: dev-***@commons.apache.org