You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "aherbert (via GitHub)" <gi...@apache.org> on 2023/07/10 13:10:03 UTC
[GitHub] [commons-statistics] aherbert commented on a diff in pull request #46: [STATISTICS-71]: Add base interfaces for all statistic implementations.
aherbert commented on code in PR #46:
URL: https://github.com/apache/commons-statistics/pull/46#discussion_r1258115764
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.</p>
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.</p>
+ */
+public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> {
+
+ /**
+ * Helper function that returns a new instance of {@link UnsupportedOperationException}.
+ * @return a new {@code UnsupportedOperationException} instance
+ */
+ static UnsupportedOperationException uoe() {
Review Comment:
This saves a few characters of typing. The result however obfuscates the stack trace. I would drop the static method and throw the exception at the correct location. However since I would also drop the immutable class this is not required anymore.
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
Review Comment:
Place an empty line here in the javadoc before the new paragraph.
The style in this component is to not close `<p>` tags. Although not valid HTML 5 it is parsed correctly by the javadoc tool and a style used through the OpenJDK source code.
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.</p>
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.</p>
+ */
+public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> {
+
+ /**
+ * Helper function that returns a new instance of {@link UnsupportedOperationException}.
+ * @return a new {@code UnsupportedOperationException} instance
+ */
+ static UnsupportedOperationException uoe() {
+ return new UnsupportedOperationException();
+ }
+
+ /**
+ * {@code Min} implementation that does not store the input value(s) processed so far.
+ */
+ private static final class StorelessMin extends Min {
+
+ /**Current min. */
+ private double min;
+
+ /**
+ * Create a StorelessMin instance.
+ */
+ StorelessMin() {
+ min = Double.POSITIVE_INFINITY;
+ }
+
+ /**
+ * Updates the state of Min statistic to reflect the addition of the new value.
+ * @param value the new value.
Review Comment:
It is not *the* new value, it is simply another value. This can be simplified to:
```
* Updates the state of the statistic to reflect the addition of {@code value}.
* @param value Value.
```
As such on an internal class it is not adding much as you have to read the code to see this private comment. If you wish this to be seen in the documentation then you should promote the method to an abstract method in the `Min` class.
You can see what is documented using:
```
mvn javadoc:javadoc
open target/site/apidocs/org/apache/commons/statistics/descriptive/Min.html
```
Here the documentation is sparse. Try promoting the method as an abstract documented method in Min and note the difference.
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.</p>
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.</p>
+ */
+public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> {
+
+ /**
+ * Helper function that returns a new instance of {@link UnsupportedOperationException}.
+ * @return a new {@code UnsupportedOperationException} instance
+ */
+ static UnsupportedOperationException uoe() {
+ return new UnsupportedOperationException();
+ }
+
+ /**
+ * {@code Min} implementation that does not store the input value(s) processed so far.
+ */
+ private static final class StorelessMin extends Min {
+
+ /**Current min. */
Review Comment:
Missing a space: `/** Current`
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.</p>
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.</p>
+ */
+public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> {
Review Comment:
This class should have a package-private constructor.
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.</p>
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.</p>
+ */
+public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> {
+
+ /**
+ * Helper function that returns a new instance of {@link UnsupportedOperationException}.
+ * @return a new {@code UnsupportedOperationException} instance
+ */
+ static UnsupportedOperationException uoe() {
+ return new UnsupportedOperationException();
+ }
+
+ /**
+ * {@code Min} implementation that does not store the input value(s) processed so far.
+ */
+ private static final class StorelessMin extends Min {
+
+ /**Current min. */
+ private double min;
+
+ /**
+ * Create a StorelessMin instance.
+ */
+ StorelessMin() {
+ min = Double.POSITIVE_INFINITY;
+ }
+
+ /**
+ * Updates the state of Min statistic to reflect the addition of the new value.
+ * @param value the new value.
+ */
+ @Override
+ public void accept(double value) {
+ min = Double.min(min, value);
+ }
+
+ @Override
+ public double getAsDouble() {
+ return min;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public <U extends DoubleStatisticAccumulator<Min>> void combine(U other) {
+ final Min otherMin = other.getDoubleStatistic();
+ accept(otherMin.getAsDouble());
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Min getDoubleStatistic() {
+ return this;
+ }
+ }
+
+ /**
+ * Immutable {@code Min} implementation.
+ */
+ private static final class ImmutableMin extends Min {
+
+ /**Min delegate. */
+ private final Min delegate;
+
+ /**
+ * Initializes the Min delegate.
+ * @param delegate Min delegate
+ */
+ ImmutableMin(final Min delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void accept(double value) {
+ throw uoe();
+ }
+
+ @Override
+ public double getAsDouble() {
+ return delegate.getAsDouble();
+ }
+
+ @Override
+ public <U extends DoubleStatisticAccumulator<Min>> void combine(U other) {
+ throw uoe();
+ }
+
+ @Override
+ public Min getDoubleStatistic() {
+ return this;
+ }
+ }
+
+ /**
+ * Creates a {@code Min} implementation which does not store the input value(s) it consumes.
+ *
+ * @return {@code Min} implementation that does not store the input value(s) processed.
+ */
+ public static Min createStoreless() {
+ return new StorelessMin();
+ }
+
+ /**
+ * Returns an immutable {@code Min} object that has the minimum of all the input value(s).
+ *
+ * @param values the input values for which we would need to compute the minimum
+ * @return Immutable Min implementation that does not store the input value(s) processed.
+ */
+ public static Min of(double... values) {
Review Comment:
At present I do not see the requirement for the returned value to be immutable. This is a specialisation. For the general use case a mutable return value is more flexible.
Consider this:
```java
double[][] a = {{1, 2}, {3, 4}};
double m1 = Arrays.stream(a)
.map(Min::of)
.reduce((x, y) -> {
x.combine(y);
return x;
}).get().getAsDouble();
```
This does highlight an issue with the `DoubleStatisticAccumulator.combine` returning void not being compatible with the `reduce` method using a method reference. One solution is to have `combine` return `DoubleStatisticAccumulator<T>`. This leads to the clunky:
```java
double m2 = Arrays.stream(a)
.map(x -> (DoubleStatisticAccumulator<Min>) Min.of(x))
.reduce(DoubleStatisticAccumulator::combine)
.get().getDoubleStatistic().getAsDouble();
```
Another option is to not bother supporting the reduce method and use:
```java
double m3 = Arrays.stream(a)
.flatMapToDouble(Arrays::stream)
.collect(Min::createStoreless, Min::accept, Min::combine)
.getAsDouble();
```
The flatmap of the stream may be less efficient that immediately creating a stream of Min objects from the array.
Or we can update `Min` to change the return type defined in the interface allowing:
```java
// In Min
@Override
public abstract <U extends DoubleStatisticAccumulator<Min>> Min combine(U other);
double m4 = Arrays.stream(a)
.map(Min::of)
.reduce(Min::combine)
.get().getAsDouble();
```
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Arrays;
+import java.util.stream.DoubleStream;
+import java.util.stream.Stream;
+
+/**
+ * Test for {@link Min}.
+ */
+final class MinTest {
Review Comment:
Test fixtures can remove the `public` keyword as we are using JUnit 5
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Statistic.java:
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Represents a statistic being computed.
+ */
+public enum Statistic {
+ /** Represents the minimum of the input value(s). */
+ MIN,
+
+ /** Represents the maximum of the input value(s). */
+ MAX,
+
+ /** Represents the sum of the input value(s). */
+ SUM,
+
+ /** Represents the arithmetic mean of the input value(s). */
+ MEAN,
+
+ /** Represents the sum of the natural logs of the input value(s). */
+ SUM_OF_LOGS,
+
+ /** Represents the sum of the squares of the input value(s). */
+ SUM_OF_SQUARES,
+
+ /** Represents the sample variance of the input value(s). */
+ VARIANCE,
+
+ /** Represents the population variance of the input value(s). */
+ POPULATION_VARIANCE,
+
+ /** Represents the standard deviation of the input value(s). */
+ STANDARD_DEVIATION,
Review Comment:
If you have sample and population variance then you should have the same for the standard deviation. Alternatively leave it to only 1 variance and plan to extend the options for a statistic with configuration of implementations (e.g. on creation).
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Arrays;
+import java.util.stream.DoubleStream;
+import java.util.stream.Stream;
+
+/**
+ * Test for {@link Min}.
+ */
+final class MinTest {
+
+ @Test
+ public void testEmpty() {
+ Min min = Min.createStoreless();
+ Min immutable = Min.of();
+
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble());
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, immutable.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testMin(double[] values, double expectedMin) {
+ Min stat = Min.createStoreless();
+ for (final double value: values) {
+ stat.accept(value);
+ }
+ double actualMin = stat.getAsDouble();
+ Assertions.assertEquals(expectedMin, actualMin, "min");
+ Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min");
+ }
+
+ static Stream<Arguments> testMin() {
+ return Stream.of(
+ Arguments.of(new double[] {3.14}, 3.14),
+ Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {-0.0, +0.0}, -0.0),
+ Arguments.of(new double[] {0.0, -0.0}, -0.0),
+ Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012),
+ Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747)
+ );
+ }
+
+ @Test
+ public void testMinImmutable() {
+ Min stat = Min.of(1.0, 2.0, -1.0, 4.0);
+ Assertions.assertThrows(UnsupportedOperationException.class, () -> stat.accept(3.14),
+ "cannot update an immutable instance");
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testCombine(double[] first, double[] second, double expectedMin) {
+ Min firstMin = Min.createStoreless();
+ Min secondMin = Min.createStoreless();
+
+ Arrays.stream(first).forEach(firstMin);
+ Arrays.stream(second).forEach(secondMin);
+
+ firstMin.combine(secondMin);
+ Assertions.assertEquals(expectedMin, firstMin.getAsDouble());
Review Comment:
You should have an assertion that secondMin is unchanged by the combine operation.
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.</p>
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.</p>
+ */
+public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> {
+
+ /**
+ * Helper function that returns a new instance of {@link UnsupportedOperationException}.
+ * @return a new {@code UnsupportedOperationException} instance
+ */
+ static UnsupportedOperationException uoe() {
+ return new UnsupportedOperationException();
+ }
+
+ /**
+ * {@code Min} implementation that does not store the input value(s) processed so far.
+ */
+ private static final class StorelessMin extends Min {
+
+ /**Current min. */
+ private double min;
+
+ /**
+ * Create a StorelessMin instance.
+ */
+ StorelessMin() {
+ min = Double.POSITIVE_INFINITY;
+ }
+
+ /**
+ * Updates the state of Min statistic to reflect the addition of the new value.
+ * @param value the new value.
+ */
+ @Override
+ public void accept(double value) {
+ min = Double.min(min, value);
+ }
+
+ @Override
+ public double getAsDouble() {
+ return min;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public <U extends DoubleStatisticAccumulator<Min>> void combine(U other) {
+ final Min otherMin = other.getDoubleStatistic();
+ accept(otherMin.getAsDouble());
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Min getDoubleStatistic() {
+ return this;
+ }
+ }
+
+ /**
+ * Immutable {@code Min} implementation.
+ */
+ private static final class ImmutableMin extends Min {
+
+ /**Min delegate. */
+ private final Min delegate;
+
+ /**
+ * Initializes the Min delegate.
+ * @param delegate Min delegate
+ */
+ ImmutableMin(final Min delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void accept(double value) {
+ throw uoe();
+ }
+
+ @Override
+ public double getAsDouble() {
+ return delegate.getAsDouble();
+ }
+
+ @Override
+ public <U extends DoubleStatisticAccumulator<Min>> void combine(U other) {
+ throw uoe();
+ }
+
+ @Override
+ public Min getDoubleStatistic() {
+ return this;
+ }
+ }
+
+ /**
+ * Creates a {@code Min} implementation which does not store the input value(s) it consumes.
+ *
+ * @return {@code Min} implementation that does not store the input value(s) processed.
+ */
+ public static Min createStoreless() {
Review Comment:
I would change this to `create()`. Any future implementations can overload create with implementation choices.
You should document the special case of the result value when no values have been added, i.e. that min=+inf.
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Arrays;
+import java.util.stream.DoubleStream;
+import java.util.stream.Stream;
+
+/**
+ * Test for {@link Min}.
+ */
+final class MinTest {
+
+ @Test
+ public void testEmpty() {
+ Min min = Min.createStoreless();
+ Min immutable = Min.of();
+
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble());
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, immutable.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testMin(double[] values, double expectedMin) {
+ Min stat = Min.createStoreless();
+ for (final double value: values) {
+ stat.accept(value);
+ }
+ double actualMin = stat.getAsDouble();
+ Assertions.assertEquals(expectedMin, actualMin, "min");
+ Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min");
+ }
+
+ static Stream<Arguments> testMin() {
+ return Stream.of(
+ Arguments.of(new double[] {3.14}, 3.14),
+ Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {-0.0, +0.0}, -0.0),
+ Arguments.of(new double[] {0.0, -0.0}, -0.0),
+ Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012),
+ Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747)
+ );
+ }
+
+ @Test
+ public void testMinImmutable() {
+ Min stat = Min.of(1.0, 2.0, -1.0, 4.0);
+ Assertions.assertThrows(UnsupportedOperationException.class, () -> stat.accept(3.14),
+ "cannot update an immutable instance");
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testCombine(double[] first, double[] second, double expectedMin) {
+ Min firstMin = Min.createStoreless();
+ Min secondMin = Min.createStoreless();
+
+ Arrays.stream(first).forEach(firstMin);
+ Arrays.stream(second).forEach(secondMin);
+
+ firstMin.combine(secondMin);
+ Assertions.assertEquals(expectedMin, firstMin.getAsDouble());
+
+ secondMin.combine(firstMin);
Review Comment:
Note here that `firstMin` has been changed to be the min, irrespective of whether first or second contained the minimum. So the second combine tests that this value is passed on to secondMin. This is redundant if you set up your arguments to target all conditions with the first combine.
Ideally you wish to test 3 conditions where one or other of the pair are the minimum:
```
A(min) + B(not-min)
A(not min) + B(min)
A(min) + B(min)
```
Plus:
```
A(empty)+B(empty)
A(not-empty)+B(empty)
A(empty)+B(not-empty)
```
You can also add arrays with NaNs in there.
I would create the conditions in the stream of the arguments. Inside the test method then test a+b or b+a, with new objects for each combine, and test the RHS argument is unchanged by the combine.
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Arrays;
+import java.util.stream.DoubleStream;
+import java.util.stream.Stream;
+
+/**
+ * Test for {@link Min}.
+ */
+final class MinTest {
+
+ @Test
+ public void testEmpty() {
+ Min min = Min.createStoreless();
+ Min immutable = Min.of();
+
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble());
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, immutable.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testMin(double[] values, double expectedMin) {
+ Min stat = Min.createStoreless();
+ for (final double value: values) {
+ stat.accept(value);
+ }
+ double actualMin = stat.getAsDouble();
+ Assertions.assertEquals(expectedMin, actualMin, "min");
+ Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min");
+ }
+
+ static Stream<Arguments> testMin() {
+ return Stream.of(
+ Arguments.of(new double[] {3.14}, 3.14),
+ Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {-0.0, +0.0}, -0.0),
+ Arguments.of(new double[] {0.0, -0.0}, -0.0),
+ Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012),
+ Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747)
+ );
+ }
+
+ @Test
+ public void testMinImmutable() {
+ Min stat = Min.of(1.0, 2.0, -1.0, 4.0);
+ Assertions.assertThrows(UnsupportedOperationException.class, () -> stat.accept(3.14),
+ "cannot update an immutable instance");
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testCombine(double[] first, double[] second, double expectedMin) {
+ Min firstMin = Min.createStoreless();
+ Min secondMin = Min.createStoreless();
+
+ Arrays.stream(first).forEach(firstMin);
+ Arrays.stream(second).forEach(secondMin);
+
+ firstMin.combine(secondMin);
+ Assertions.assertEquals(expectedMin, firstMin.getAsDouble());
+
+ secondMin.combine(firstMin);
+ Assertions.assertEquals(expectedMin, secondMin.getAsDouble());
+ }
+
+ static Stream<Arguments> testCombine() {
+ return Stream.of(
+ Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718),
+ Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1),
+ Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0),
+ Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79),
+ Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0),
+ Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY},
+ new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY},
+ new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE},
+ Double.NaN)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testParallelStream(double[] values, double expectedMin) {
+ double actualMin = Arrays.stream(values).parallel().collect(Min::createStoreless, Min::accept, Min::combine).getAsDouble();
+ Assertions.assertEquals(expectedMin, actualMin);
+
+ DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN);
Review Comment:
This test should be its own fixture since it does not use the test parameters.
##########
commons-statistics-descriptive/src/test/java/org/apache/commons/statistics/descriptive/MinTest.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Arrays;
+import java.util.stream.DoubleStream;
+import java.util.stream.Stream;
+
+/**
+ * Test for {@link Min}.
+ */
+final class MinTest {
+
+ @Test
+ public void testEmpty() {
+ Min min = Min.createStoreless();
+ Min immutable = Min.of();
+
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, min.getAsDouble());
+ Assertions.assertEquals(Double.POSITIVE_INFINITY, immutable.getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testMin(double[] values, double expectedMin) {
+ Min stat = Min.createStoreless();
+ for (final double value: values) {
+ stat.accept(value);
+ }
+ double actualMin = stat.getAsDouble();
+ Assertions.assertEquals(expectedMin, actualMin, "min");
+ Assertions.assertEquals(expectedMin, Min.of(values).getAsDouble(), "min");
+ }
+
+ static Stream<Arguments> testMin() {
+ return Stream.of(
+ Arguments.of(new double[] {3.14}, 3.14),
+ Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0),
+ Arguments.of(new double[] {Double.NaN, 3.14, Double.NaN, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {-0.0, +0.0}, -0.0),
+ Arguments.of(new double[] {0.0, -0.0}, -0.0),
+ Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012),
+ Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747)
+ );
+ }
+
+ @Test
+ public void testMinImmutable() {
+ Min stat = Min.of(1.0, 2.0, -1.0, 4.0);
+ Assertions.assertThrows(UnsupportedOperationException.class, () -> stat.accept(3.14),
+ "cannot update an immutable instance");
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testCombine(double[] first, double[] second, double expectedMin) {
+ Min firstMin = Min.createStoreless();
+ Min secondMin = Min.createStoreless();
+
+ Arrays.stream(first).forEach(firstMin);
+ Arrays.stream(second).forEach(secondMin);
+
+ firstMin.combine(secondMin);
+ Assertions.assertEquals(expectedMin, firstMin.getAsDouble());
+
+ secondMin.combine(firstMin);
+ Assertions.assertEquals(expectedMin, secondMin.getAsDouble());
+ }
+
+ static Stream<Arguments> testCombine() {
+ return Stream.of(
+ Arguments.of(new double[] {3.14}, new double[] {2.718}, 2.718),
+ Arguments.of(new double[] {3.14, 1.1, 22.22}, new double[] {2.718, 1.1, 333.333}, 1.1),
+ Arguments.of(new double[] {12.34, 56.78, -2.0}, new double[] {0.0, 23.45}, -2.0),
+ Arguments.of(new double[] {-2023.79, 11.11, 333.333}, new double[] {1.1}, -2023.79),
+ Arguments.of(new double[] {1.1, +0.0, 3.14}, new double[] {22.22, 2.718, -0.0}, -0.0),
+ Arguments.of(new double[] {0.0, -Double.MIN_VALUE, Double.POSITIVE_INFINITY},
+ new double[] {Double.NEGATIVE_INFINITY, -0.0, Double.NEGATIVE_INFINITY, Double.MIN_VALUE},
+ Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {0.0, Double.NaN, -Double.MIN_VALUE, Double.POSITIVE_INFINITY},
+ new double[] {Double.NaN, -0.0, Double.NaN, Double.NEGATIVE_INFINITY, Double.MIN_VALUE},
+ Double.NaN)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testParallelStream(double[] values, double expectedMin) {
+ double actualMin = Arrays.stream(values).parallel().collect(Min::createStoreless, Min::accept, Min::combine).getAsDouble();
+ Assertions.assertEquals(expectedMin, actualMin);
+
+ DoubleStream nanStream = DoubleStream.of(Double.NaN, Double.NaN, Double.NaN);
+ Assertions.assertTrue(Double.isNaN(nanStream.parallel().collect(Min::createStoreless, Min::accept, Min::combine).getAsDouble()));
+ }
+
+ static Stream<Arguments> testParallelStream() {
+ return Stream.of(
+ Arguments.of(new double[] {3.14}, 3.14),
+ Arguments.of(new double[] {12.34, 56.78, -2.0}, -2.0),
+ Arguments.of(new double[] {1.2, -34.56, 456.789, -5678.9012}, -5678.9012),
+ Arguments.of(new double[] {-23467824, 23648, 2368, 23749, -23424, -23492, -92397747}, -92397747),
+ Arguments.of(new double[] {-1d, 1d, Double.NaN}, Double.NaN),
+ Arguments.of(new double[] {+0.0d, -0.0d, 1.0, 3.14}, -0.0d),
+ Arguments.of(new double[] {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NEGATIVE_INFINITY),
+ Arguments.of(new double[] {0.0d, Double.NaN, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}, Double.NaN)
+ );
+ }
+
+ @Test
+ public void testCombineImmutable() {
+ Min immutable = Min.of(1.1, 22.22, -333.333);
+ Min mutable = Min.createStoreless();
+ mutable.combine(immutable);
+ Assertions.assertEquals(-333.333, mutable.getAsDouble());
+ Assertions.assertEquals(-333.333, immutable.getAsDouble());
+
+ Min mutable2 = Min.createStoreless();
+ mutable2.accept(-4444.4444);
+ mutable2.combine(immutable);
+ Assertions.assertEquals(-4444.4444, mutable2.getAsDouble());
+
+ Assertions.assertThrows(UnsupportedOperationException.class, () -> immutable.combine(mutable),
+ "cannot update an immutable instance");
+ }
+
+ @Test
+ public void testSpecialValues() {
+ double[] testArray = {0.0d, +0.0d, -0.0d, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY};
+ Min stat = Min.createStoreless();
+
+ stat.accept(testArray[0]);
+ Assertions.assertEquals(0.0d, stat.getAsDouble());
+
+ stat.accept(testArray[1]);
+ Assertions.assertEquals(0.0d, stat.getAsDouble());
+
+ stat.accept(testArray[2]);
+ Assertions.assertEquals(-0.0d, stat.getAsDouble());
+
+ stat.accept(testArray[3]);
+ Assertions.assertEquals(-0.0d, stat.getAsDouble());
+
+ stat.accept(testArray[4]);
+ Assertions.assertEquals(Double.NEGATIVE_INFINITY, stat.getAsDouble());
+
+ Assertions.assertEquals(Double.NEGATIVE_INFINITY, Min.of(testArray).getAsDouble());
+ }
+
+ @ParameterizedTest
+ @MethodSource
+ public void testNaNs(double[] values, double expectedMin) {
+ Min stat = Min.createStoreless();
+ for (final double value: values) {
+ stat.accept(value);
+ }
+ Assertions.assertEquals(expectedMin, stat.getAsDouble());
+
+ Assertions.assertTrue(Double.isNaN(Min.of(Double.NaN, Double.NaN, Double.NaN).getAsDouble()));
Review Comment:
Move to a difference test fixture as it does not use the arguments.
##########
commons-statistics-descriptive/src/main/java/org/apache/commons/statistics/descriptive/Min.java:
##########
@@ -0,0 +1,149 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.statistics.descriptive;
+
+/**
+ * Returns the minimum of the available values.
+ *
+ * <p>The result is <code>NaN</code> if any of the values is <code>NaN</code>.</p>
+ * <p>This class is designed to work with (though does not require)
+ * {@linkplain java.util.stream streams}.
+ *
+ * <p><strong>This implementation is not thread safe.</strong>
+ * If multiple threads access an instance of this class concurrently,
+ * and at least one of the threads invokes the <code>accept()</code> or
+ * <code>combine()</code> method, it must be synchronized externally.</p>
+ * <p>However, it is safe to use <code>accept()</code> and <code>combine()</code>
+ * as <code>accumulator</code> and <code>combiner</code> functions of
+ * {@link java.util.stream.Collector Collector} on a parallel stream,
+ * because the parallel implementation of {@link java.util.stream.Stream#collect Stream.collect()}
+ * provides the necessary partitioning, isolation, and merging of results for
+ * safe and efficient parallel execution.</p>
+ */
+public abstract class Min implements DoubleStatistic, DoubleStatisticAccumulator<Min> {
+
+ /**
+ * Helper function that returns a new instance of {@link UnsupportedOperationException}.
+ * @return a new {@code UnsupportedOperationException} instance
+ */
+ static UnsupportedOperationException uoe() {
+ return new UnsupportedOperationException();
+ }
+
+ /**
+ * {@code Min} implementation that does not store the input value(s) processed so far.
+ */
+ private static final class StorelessMin extends Min {
+
+ /**Current min. */
+ private double min;
+
+ /**
+ * Create a StorelessMin instance.
+ */
+ StorelessMin() {
+ min = Double.POSITIVE_INFINITY;
+ }
+
+ /**
+ * Updates the state of Min statistic to reflect the addition of the new value.
+ * @param value the new value.
+ */
+ @Override
+ public void accept(double value) {
+ min = Double.min(min, value);
+ }
+
+ @Override
+ public double getAsDouble() {
+ return min;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public <U extends DoubleStatisticAccumulator<Min>> void combine(U other) {
+ final Min otherMin = other.getDoubleStatistic();
+ accept(otherMin.getAsDouble());
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Min getDoubleStatistic() {
+ return this;
+ }
+ }
+
+ /**
+ * Immutable {@code Min} implementation.
+ */
+ private static final class ImmutableMin extends Min {
+
+ /**Min delegate. */
+ private final Min delegate;
+
+ /**
+ * Initializes the Min delegate.
+ * @param delegate Min delegate
+ */
+ ImmutableMin(final Min delegate) {
+ this.delegate = delegate;
+ }
+
+ @Override
+ public void accept(double value) {
+ throw uoe();
+ }
+
+ @Override
+ public double getAsDouble() {
+ return delegate.getAsDouble();
+ }
+
+ @Override
+ public <U extends DoubleStatisticAccumulator<Min>> void combine(U other) {
+ throw uoe();
+ }
+
+ @Override
+ public Min getDoubleStatistic() {
+ return this;
+ }
+ }
+
+ /**
+ * Creates a {@code Min} implementation which does not store the input value(s) it consumes.
+ *
+ * @return {@code Min} implementation that does not store the input value(s) processed.
+ */
+ public static Min createStoreless() {
+ return new StorelessMin();
+ }
+
+ /**
+ * Returns an immutable {@code Min} object that has the minimum of all the input value(s).
Review Comment:
Again, document the value if the input is an empty array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org