Add greaterThan, lessThan, greaterOrEqualTo, lessOrEqualTo convenience wrappers
Opened this issue · 8 comments
I'd like to bring forth the aforementioned methods as convenience wrappers for the rather unwieldy and space-consuming compareTo(). Since we already have convenience methods min() and max()
I can also take care of this implementation if necessary, just wanted to post this as a suggestion to know what others think.
The methods would be instance-based and take a single Apcomplex / Apint / Apfloat etc as their argument, returning a boolean.
I somewhat fail to see how this is more convenient and consumes less space
a.greaterOrEqualTo(b)
than
a.compareTo(b) >= 0
BigInteger and BigDecimal do not have such methods either. Also in some cases it consumes more space, not less (21 vs. 19 characters in this example).
The min and max methods are not member functions in the Apfloat etc. classes but static methods in ApfloatMath etc. If the methods were implemented in a similar way to min and max e.g.
ApfloatMath.greaterOrEqualTo(a, b)
the idea is even worse, but this is probably not what you meant.
Maybe when these methods are added to the BigInteger and BigDecimal classes, they can be added to apfloat as well. But not until that.
First, I wanted to thank you for responding; it means a lot to me to be heard and have suggestions considered. :)
You are correct, the wrapper methods would consume slightly more horizontal space in the worst case. In the best case, (a.lessThan(b) is shorter than a.compareTo(b) < 0 (13 characters vs 18), but the differences are rather minor in either direction, being within ~5 characters for most (all?) cases. I think it's overall not an active harm to code verbosity to have the wrapper methods named as such.
You're also correct in that I meant I originally conceived these methods to be instance methods, rather than the static min() and max() methods of ApfloatMath (and friends). I apologize for my poor communication. I also agree that it doesn't make much sense to have these methods be static within ApfloatMath et. al.
The main benefit I saw to suggesting these methods was grokability; in my opinion, if(a.greaterOrEqualTo(b)) is a much simpler grok than if(a.compareTo(b) >= 0), since the developer doesn't need to mentally remember in which direction the comparison is being made whenever compareTo is called. In my personal experience, I've had to make many corrections to my compareTo method calls, in part due to my lack of diligence when dealing with compareTo methods, and still often find myself having to stop and think about it for a second. I may be the only one who experiences this problem.
Is there some technicality which prevents implementation here due to not being present in BigInteger / BigDecimal? I am likely less familiar with the libraries than you, only having used them briefly a year or so ago. If there's no hard dependency, could you elaborate on why we would wait until their respective implementations?
Why not using the typically shortcuts used in other programming languages?
Because it should return a boolean true or false you can call the methods isLT(), isLE(), isGT(), isGE()?
I don't oppose this proposal because of the technical implementation. The implementation would be trivial.
The compareTo method is defined by the Comparable interface in the JDK. There are some 200 classes in the JDK alone that implement this interface, and e.g. in all the Maven libraries in existence, there must be thousands of other classes implementing it. The compareTo method may be "unwieldy" but it is the method that millions of Java developers know. They know what it does and how to use it, without checking the docs. Adding another method that essentially does the same thing but with a different syntax is not an improvement. Developers would have to first learn of this method and then learn to use it. It could be argued that adding such a method does not improve things; in fact it does the opposite: it makes the library more difficult to use. It bloats the API and increases the threshold to take the library to use.
This is the "less is more" approach, which was popular at least during the early days of Java. I know that some people have the opposing view ("more is more") and want to add every possible method to every API just for syntactic sugar. It is of course a valid point of view but I don't support it, at least not in this case.
There is no particular reason to wait for such a method to be added to BigInteger or BigDecimal. The apfloat library in general tries to implement the same methods that exist in BigInteger and BigDecimal, to be more useful. So, if these methods were added to BigInteger or BigDecimal then that would be a reason to add them to the apfloat library as well.
But to generalize this idea, why wait for the methods to be added to BigInteger or BigDecimal. These methods could be as well added as default methods to the java.lang.Comparable interface. Then it would readily work, not just with the apfloat library, but all classes that implement Comparable. If you are interested in pursuing this path, why not submit a JEP or JSR for it?
I have difficulties in understanding your argument about remembering in which direction the comparison is being made in compareTo. At least I use this mnemonic:
- First of all, in practically all code, the result of
compareTois compared against zero (this is obvious from the specification of this method) - Move the operator in place of the text "compareTo"
- Discard the zero
a.compareTo(b) > 0 ⇔ a > b
a.compareTo(b) >= 0 ⇔ a >= b
a.compareTo(b) < 0 ⇔ a < b
a.compareTo(b) <= 0 ⇔ a <= b
With this mnemonic the order of comparison is clear, at least to me.
In the apfloat source code I also often see things like
signum() < 0
signum() == 0
signum() > 0
signum () >= 0
signum() <= 0
I suggest to extract methods which much clearer describe the behaviour:
isNegative()
isZero()
isPositive()
isNonNegative()
isNonPositive()
Maybe if BigDecimal or BigInteger had these methods then there would be a reason to add them (because of similarity if APIs), but otherwise I don't think it makes sense to add them for syntactic sugar. It would be just clutter. Less is more.
Ok, from comparing the apfloat numbers API with the Java numbers API you are correct.
isZero() was added to release 1.13.0 because it's actually useful for complex numbers (without it you have to do two checks: to check if the real part signum is zero and if the imaginary part signum is zero).