I am interested in moving forward on making the Fraction classes VALJOs
[NUMBERS-75]. Just a preliminary question for now, are we otherwise happy with the Fraction class in the context of commons-numbers? Or should I look around for any odd behaviors leftover from commons-math (Complex had a lot of those) that might also be improved? Eric |
On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote:
> I am interested in moving forward on making the Fraction classes > VALJOs > [NUMBERS-75]. > > Just a preliminary question for now, are we otherwise happy with the > Fraction class in the context of commons-numbers? Or should I look > around > for any odd behaviors leftover from commons-math (Complex had a lot > of > 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 > > Eric --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
The Fraction class is IMO looking good (in better shape than Complex was
in) and is already quite close to fulfilling the standards for a VALJO. Equals() and CompareTo() are well designed and consistent. I see two missing 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 think a decent schema emerges: current constructor -> proposed factory method -------------------------------------------------------- public Fraction(double value) -> public fromDouble(double value) public Fraction(double value, double epsilon, int maxIterations) -> public 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) so this is what I propose to go with. If disambiguation in the double cases is still a problem, the second and third of the double constructors could be fromDoubleEpsMaxInt and fromDoubleMaxDenom . Eric On Thu, Oct 11, 2018 at 7:00 AM Gilles <[hidden email]> wrote: > On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote: > > I am interested in moving forward on making the Fraction classes > > VALJOs > > [NUMBERS-75]. > > > > Just a preliminary question for now, are we otherwise happy with the > > Fraction class in the context of commons-numbers? Or should I look > > around > > for any odd behaviors leftover from commons-math (Complex had a lot > > of > > 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 > > > > > Eric > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > > |
On Tue, 16 Oct 2018 16:55:02 -0700, Eric Barnhill wrote:
> The Fraction class is IMO looking good (in better shape than Complex > was > in) and is already quite close to fulfilling the standards for a > VALJO. > Equals() and CompareTo() are well designed and consistent. I see two > missing 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 > think a decent schema emerges: > > current constructor -> proposed factory method > -------------------------------------------------------- > public Fraction(double value) -> public fromDouble(double value) > public Fraction(double value, double epsilon, int maxIterations) -> > public > 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 > > so this is what I propose to go with. > > If disambiguation in the double cases is still a problem, the second > and > third of the double constructors could be fromDoubleEpsMaxInt and > fromDoubleMaxDenom . > > Eric > > > On Thu, Oct 11, 2018 at 7:00 AM Gilles <[hidden email]> > wrote: > >> On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote: >> > I am interested in moving forward on making the Fraction classes >> > VALJOs >> > [NUMBERS-75]. >> > >> > Just a preliminary question for now, are we otherwise happy with >> the >> > Fraction class in the context of commons-numbers? Or should I look >> > around >> > for any odd behaviors leftover from commons-math (Complex had a >> lot >> > of >> > 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 >> >> > >> > Eric >> --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
Oh 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, so 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. On Tue, Oct 16, 2018 at 5:56 PM Gilles <[hidden email]> wrote: > On Tue, 16 Oct 2018 16:55:02 -0700, Eric Barnhill wrote: > > The Fraction class is IMO looking good (in better shape than Complex > > was > > in) and is already quite close to fulfilling the standards for a > > VALJO. > > Equals() and CompareTo() are well designed and consistent. I see two > > missing 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 > > think a decent schema emerges: > > > > current constructor -> proposed factory method > > -------------------------------------------------------- > > public Fraction(double value) -> public fromDouble(double value) > > public Fraction(double value, double epsilon, int maxIterations) -> > > public > > 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 > > > > > so this is what I propose to go with. > > > > If disambiguation in the double cases is still a problem, the second > > and > > third of the double constructors could be fromDoubleEpsMaxInt and > > fromDoubleMaxDenom . > > > > Eric > > > > > > On Thu, Oct 11, 2018 at 7:00 AM Gilles <[hidden email]> > > wrote: > > > >> On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote: > >> > I am interested in moving forward on making the Fraction classes > >> > VALJOs > >> > [NUMBERS-75]. > >> > > >> > Just a preliminary question for now, are we otherwise happy with > >> the > >> > Fraction class in the context of commons-numbers? Or should I look > >> > around > >> > for any odd behaviors leftover from commons-math (Complex had a > >> lot > >> > of > >> > 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 > >> > >> > > >> > Eric > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > > |
On Wed, 17 Oct 2018 16:33:58 -0700, Eric Barnhill wrote:
> Oh 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 > so 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. > > On Tue, Oct 16, 2018 at 5:56 PM Gilles <[hidden email]> > wrote: > >> On Tue, 16 Oct 2018 16:55:02 -0700, Eric Barnhill wrote: >> > The Fraction class is IMO looking good (in better shape than >> Complex >> > was >> > in) and is already quite close to fulfilling the standards for a >> > VALJO. >> > Equals() and CompareTo() are well designed and consistent. I see >> two >> > missing 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 >> > think a decent schema emerges: >> > >> > current constructor -> proposed factory method >> > -------------------------------------------------------- >> > public Fraction(double value) -> public fromDouble(double value) >> > public Fraction(double value, double epsilon, int maxIterations) >> -> >> > public >> > 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 >> >> > >> > so this is what I propose to go with. >> > >> > If disambiguation in the double cases is still a problem, the >> second >> > and >> > third of the double constructors could be fromDoubleEpsMaxInt and >> > fromDoubleMaxDenom . >> > >> > Eric >> > >> > >> > On Thu, Oct 11, 2018 at 7:00 AM Gilles >> <[hidden email]> >> > wrote: >> > >> >> On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote: >> >> > I am interested in moving forward on making the Fraction >> classes >> >> > VALJOs >> >> > [NUMBERS-75]. >> >> > >> >> > Just a preliminary question for now, are we otherwise happy >> with >> >> the >> >> > Fraction class in the context of commons-numbers? Or should I >> look >> >> > around >> >> > for any odd behaviors leftover from commons-math (Complex had a >> >> lot >> >> > of >> >> > 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 >> >> >> >> > >> >> > Eric >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
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 On Thu, 18 Oct 2018 at 11:45, Gilles <[hidden email]> wrote: > > On Wed, 17 Oct 2018 16:33:58 -0700, Eric Barnhill wrote: > > Oh 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 > > > so 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. > > > > On Tue, Oct 16, 2018 at 5:56 PM Gilles <[hidden email]> > > wrote: > > > >> On Tue, 16 Oct 2018 16:55:02 -0700, Eric Barnhill wrote: > >> > The Fraction class is IMO looking good (in better shape than > >> Complex > >> > was > >> > in) and is already quite close to fulfilling the standards for a > >> > VALJO. > >> > Equals() and CompareTo() are well designed and consistent. I see > >> two > >> > missing 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 > >> > think a decent schema emerges: > >> > > >> > current constructor -> proposed factory method > >> > -------------------------------------------------------- > >> > public Fraction(double value) -> public fromDouble(double value) > >> > public Fraction(double value, double epsilon, int maxIterations) > >> -> > >> > public > >> > 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 > >> > >> > > >> > so this is what I propose to go with. > >> > > >> > If disambiguation in the double cases is still a problem, the > >> second > >> > and > >> > third of the double constructors could be fromDoubleEpsMaxInt and > >> > fromDoubleMaxDenom . > >> > > >> > Eric > >> > > >> > > >> > On Thu, Oct 11, 2018 at 7:00 AM Gilles > >> <[hidden email]> > >> > wrote: > >> > > >> >> On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote: > >> >> > I am interested in moving forward on making the Fraction > >> classes > >> >> > VALJOs > >> >> > [NUMBERS-75]. > >> >> > > >> >> > Just a preliminary question for now, are we otherwise happy > >> with > >> >> the > >> >> > Fraction class in the context of commons-numbers? Or should I > >> look > >> >> > around > >> >> > for any odd behaviors leftover from commons-math (Complex had a > >> >> lot > >> >> > of > >> >> > 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 > >> >> > >> >> > > >> >> > Eric > >> >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
You are indeed and thank you very much for this feedback!
I gave up on pushing fraction through GitHub and have pushed my first round of changes via Apache. They are available on the branch fraction-dev for inspection. I will incorporate the rest of your suggestions on my next iteration. Eric On Sun, Oct 28, 2018 at 1:52 AM Stephen Colebourne <[hidden email]> wrote: > 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 > > > On Thu, 18 Oct 2018 at 11:45, Gilles <[hidden email]> wrote: > > > > On Wed, 17 Oct 2018 16:33:58 -0700, Eric Barnhill wrote: > > > Oh 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 > > > > > so 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. > > > > > > On Tue, Oct 16, 2018 at 5:56 PM Gilles <[hidden email]> > > > wrote: > > > > > >> On Tue, 16 Oct 2018 16:55:02 -0700, Eric Barnhill wrote: > > >> > The Fraction class is IMO looking good (in better shape than > > >> Complex > > >> > was > > >> > in) and is already quite close to fulfilling the standards for a > > >> > VALJO. > > >> > Equals() and CompareTo() are well designed and consistent. I see > > >> two > > >> > missing 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 > > >> > think a decent schema emerges: > > >> > > > >> > current constructor -> proposed factory method > > >> > -------------------------------------------------------- > > >> > public Fraction(double value) -> public fromDouble(double value) > > >> > public Fraction(double value, double epsilon, int maxIterations) > > >> -> > > >> > public > > >> > 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 > > >> > > >> > > > >> > so this is what I propose to go with. > > >> > > > >> > If disambiguation in the double cases is still a problem, the > > >> second > > >> > and > > >> > third of the double constructors could be fromDoubleEpsMaxInt and > > >> > fromDoubleMaxDenom . > > >> > > > >> > Eric > > >> > > > >> > > > >> > On Thu, Oct 11, 2018 at 7:00 AM Gilles > > >> <[hidden email]> > > >> > wrote: > > >> > > > >> >> On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote: > > >> >> > I am interested in moving forward on making the Fraction > > >> classes > > >> >> > VALJOs > > >> >> > [NUMBERS-75]. > > >> >> > > > >> >> > Just a preliminary question for now, are we otherwise happy > > >> with > > >> >> the > > >> >> > Fraction class in the context of commons-numbers? Or should I > > >> look > > >> >> > around > > >> >> > for any odd behaviors leftover from commons-math (Complex had a > > >> >> lot > > >> >> > of > > >> >> > 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 > > >> >> > > >> >> > > > >> >> > Eric > > >> >> > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [hidden email] > > For additional commands, e-mail: [hidden email] > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [hidden email] > For additional commands, e-mail: [hidden email] > > |
In reply to this post by jodastephen
Hi.
On Sun, 28 Oct 2018 08:52:01 +0000, Stephen Colebourne wrote: > As the author of the blog and term VALJO, here are some comments on > Fraction: Thanks for the review and comments. > 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). I'm not convinced that it is useful to make a difference between construction and conversion... > 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. +1 > The method order in the class is strange. It seems to be very old Commons Math code... > 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()`. At first sight, "reduce" should be private. [I've added a note in JIRA that this class should be reviewed before the release.] > > 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 Where is this convention used/defined? What is the rationale/advantage? > No doubt this would apply widely in the project Sure, but we should you then raise the issue of adoption by *all* Commons components. Best, Gilles > HTH > Stephen > > > On Thu, 18 Oct 2018 at 11:45, Gilles <[hidden email]> > wrote: >> >> On Wed, 17 Oct 2018 16:33:58 -0700, Eric Barnhill wrote: >> > Oh 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 >> >> > so 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. >> > >> > On Tue, Oct 16, 2018 at 5:56 PM Gilles >> <[hidden email]> >> > wrote: >> > >> >> On Tue, 16 Oct 2018 16:55:02 -0700, Eric Barnhill wrote: >> >> > The Fraction class is IMO looking good (in better shape than >> >> Complex >> >> > was >> >> > in) and is already quite close to fulfilling the standards for >> a >> >> > VALJO. >> >> > Equals() and CompareTo() are well designed and consistent. I >> see >> >> two >> >> > missing 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 >> >> > think a decent schema emerges: >> >> > >> >> > current constructor -> proposed factory method >> >> > -------------------------------------------------------- >> >> > public Fraction(double value) -> public fromDouble(double >> value) >> >> > public Fraction(double value, double epsilon, int >> maxIterations) >> >> -> >> >> > public >> >> > 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 >> >> >> >> > >> >> > so this is what I propose to go with. >> >> > >> >> > If disambiguation in the double cases is still a problem, the >> >> second >> >> > and >> >> > third of the double constructors could be fromDoubleEpsMaxInt >> and >> >> > fromDoubleMaxDenom . >> >> > >> >> > Eric >> >> > >> >> > >> >> > On Thu, Oct 11, 2018 at 7:00 AM Gilles >> >> <[hidden email]> >> >> > wrote: >> >> > >> >> >> On Wed, 10 Oct 2018 16:18:50 -0700, Eric Barnhill wrote: >> >> >> > I am interested in moving forward on making the Fraction >> >> classes >> >> >> > VALJOs >> >> >> > [NUMBERS-75]. >> >> >> > >> >> >> > Just a preliminary question for now, are we otherwise happy >> >> with >> >> >> the >> >> >> > Fraction class in the context of commons-numbers? Or should >> I >> >> look >> >> >> > around >> >> >> > for any odd behaviors leftover from commons-math (Complex >> had a >> >> >> lot >> >> >> > of >> >> >> > 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 >> >> >> >> >> >> > >> >> >> > Eric >> >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by jodastephen
On Sun, Oct 28, 2018 at 1:52 AM Stephen Colebourne <[hidden email]>
wrote: > > The order of the static constants is also strange - I'm sure a more > logical order could be chosen. > I have rearranged the constants into the order 0, 1, 1/2, 1/3, 2/3, 1/4, 3/4 . I propose dropping the constants for fifths and negative one. One needs to draw the line somewhere reasonable. Any objections? |
Second question, is there any reason not to use syntactic sugar for
distinctions between "multiply" and " multpliedBy"? With the former being the suger, if the latter is a better standard. It seems like both sides of the fence can be easily bridged here. On Fri, Nov 2, 2018 at 4:57 PM Eric Barnhill <[hidden email]> wrote: > > > On Sun, Oct 28, 2018 at 1:52 AM Stephen Colebourne <[hidden email]> > wrote: > >> >> The order of the static constants is also strange - I'm sure a more >> logical order could be chosen. >> > > I have rearranged the constants into the order 0, 1, 1/2, 1/3, 2/3, 1/4, > 3/4 . I propose dropping the constants > for fifths and negative one. One needs to draw the line somewhere > reasonable. Any objections? > |
In reply to this post by Eric Barnhill
On Fri, 2 Nov 2018 16:57:43 -0700, Eric Barnhill wrote:
> On Sun, Oct 28, 2018 at 1:52 AM Stephen Colebourne > <[hidden email]> > wrote: > >> >> The order of the static constants is also strange - I'm sure a more >> logical order could be chosen. >> > > I have rearranged the constants into the order 0, 1, 1/2, 1/3, 2/3, > 1/4, > 3/4 . I propose dropping the constants > for fifths and negative one. One needs to draw the line somewhere > reasonable. Any objections? I think that only "zero" and "one" should be kept: The rest has no particular use or meaning within the component; better leave them to the application layer, and keep this API lean. Did you have a look at the updated description: https://issues.apache.org/jira/browse/NUMBERS-74 ? Regards, Gilles --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
In reply to this post by Eric Barnhill
On Fri, 2 Nov 2018 17:00:19 -0700, Eric Barnhill wrote:
> Second question, is there any reason not to use syntactic sugar for > distinctions between "multiply" and " > multpliedBy"? With the former being the suger, if the latter is a > better > standard. Statement needs some reference before we embark on changing names for longer ones. Cf. my previous post: https://markmail.org/message/r5hxy5f4eduxp2vn [If the rationale is "correct English", fine, but then all the modules must be modified accordingly.] > It seems like both sides of the fence can be easily bridged here. IMO, syntactic sugar should be justified by more than sparing 4 keystrokes, especially if we want to induce usage of proper spelling. Regards, Gilles > On Fri, Nov 2, 2018 at 4:57 PM Eric Barnhill <[hidden email]> > wrote: > >> >> >> On Sun, Oct 28, 2018 at 1:52 AM Stephen Colebourne >> <[hidden email]> >> wrote: >> >>> >>> The order of the static constants is also strange - I'm sure a more >>> logical order could be chosen. >>> >> >> I have rearranged the constants into the order 0, 1, 1/2, 1/3, 2/3, >> 1/4, >> 3/4 . I propose dropping the constants >> for fifths and negative one. One needs to draw the line somewhere >> reasonable. Any objections? >> --------------------------------------------------------------------- To unsubscribe, e-mail: [hidden email] For additional commands, e-mail: [hidden email] |
Free forum by Nabble | Edit this page |