Hi.
On Tue, 9 May 2017 13:29:21 +0100, Stian Soiland-Reyes wrote:
On Mon, 08 May 2017 14:16:12 +0200, Gilles
<gil...@harfang.homelinux.org> wrote:
> Overall the addition looks good and well commented. The javadoc is
a
> bit
> sparse and could do with some more hyperlinks.
Strange; I thought that it was doing rather well on this point.
Can you be more specific? [Please open a JIRA report.]
Just thought it would be good with a quick one-liner about what the
gamma function is rather than a link to Wolfram Alpha. I added
something
generic like:
* The Gamma function can be seen to extend the factorial function to
cover
* real and complex numbers, but with its argument shifted by
* {@code * -1}. This implementation supports real numbers.
It can't hurt. ;-)
(BTW - why does it not support complex numbers?)
New feature?
The original (CM) design contained in single big class and static
methods for all the functions.
I thought it would be clearer to split it into several classes (each
with its own unit test class).
Yes, this is a very good design choice, I agree with this. Part of
the
javadoc improvements I tried to add is to also then relate the new
classes to eachother, so I added say a @see Gamma back from Digamma.
Thanks.
One goal was to switch away from static methods, to allow an easy
path
to using the Java 8 functional interfaces.
[Help is most welcome to check that it will indeed be the case.]
Hm, good insentive - but I don't think this helps much.. The only way
I
could use it now is to make a new instance, which looks awkward:
import org.apache.commons.numbers.gamma.Gamma;
@Test
public void streamGamma() throws Exception {
List<Double> numbers =
Arrays.asList(-2.1,-2.0,-1.99,-1.95,-1.9,-1.8,-1.7,-1.6,-1.5,-1.4,-1.3,-1.2,-1.1,-1.0,-0.9);
Stream<Double> d = numbers.parallelStream().map(new
Gamma()::value);
d.forEachOrdered(System.out::println);
}
Passing just "new Gamma()" won't work as it does not implement any
@FunctionalInterface.
If I make the method static and rename it to "gamma", then I can do
the much
simpler:
Stream<Double> d = numbers.parallelStream().map(Gamma::gamma);
I don't like the "gamma" duplication.
I'll make the method "value" static to allow the above usage.
.. as well as import it as a static reference (however Java 8 won't
allow passing just "gamma").
I can only see this "value()" style make sense if there is also a
common
interface, perhaps DoubleFunction<Double>?
https://docs.oracle.com/javase/8/docs/api/java/util/function/DoubleFunction.html
.. but this uses apply(x).
[I specifically avoided "apply" so that it is free for when the
component
targets Java 8.]
Can the "apply" method be static?
I've fixed "Digamma" and "Trigamma" that still inadvertently
contained
static methods.
Thanks for spotting it.
Hm, but you fixed it the wrong way? :) Now they are all called
value() and are
non-static.. why do we need to make instances of these classes which
only state
is to other static helper classes?
Won't a private constructor (which was my original intention) preclude
some of the functional usages?
If not, I'll make the change
The "instance" field is for use "helper" in other classes in the
package,
to avoid a few unnecessary instantiations.
But I tend to agree that it is perhaps not worth it.
If the class has a public constructor there will be many
instansiations of the class.
Now it is quite hard to check the class is actually immutable, as we
have to
check these fields.
I know there many good arguments against implicit static
initializers, as it
can be quite tricky to determine the initialization order. Referring
to such
instances from static fields can cause such issues. (I don't think
that's the
case here though).
Cf. above.
I'd be willing to make this package target Java 8. [Any objections?]
+100! (All of Commons Numbers, presumably)
Not necessarily.
[Personally I don't mind, but Sebb might...]
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
