[
https://issues.apache.org/jira/browse/STATISTICS-62?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17685847#comment-17685847
]
Alex Herbert commented on STATISTICS-62:
----------------------------------------
Extending the idea of a generic builder using:
{code:java}
Builder<T> implements Supplier<T> {
Builder(Function<Builder<T>, T> constructor, EnumSet<Option> supported) {
// set-up
}
// setters & getters for all options.
// setters throw if not supported.
T get() {
return constructor.apply(this);
}
}
class MyTest {
MyTest(Builder<MyTest> b) {
// copy settings
}
static Builder<MyTest> builder() {
return new Builder<>(MyTest::new);
}
}{code}
However I do not think a builder pattern is necessary here:
# Most of the options have sensible defaults. Only a few need to be changed.
The builder pattern for constructing objects with multiple combinations of
arguments does not reduce object creation when only 2 options are changed.
# A generic builder with 7 settable options, 6 of which throw an exception,
for a class that has 1 option seems over engineered IMO.
# Minor nit: Having the setters in the builder disconnects the documentation
of the property from the test. When some options only apply to specific tests
in the Test class then it is nice to be able to document this directly in the
property setter.
# Use of a Builder should support setting no options without any object
creation. This thus requires some type of default instance method.
I will convert what I have to the 'property change to new instance' pattern and
can open a PR with the current code. This will follow the format:
{code:java}
KolmogorovSmirnovTest.withDefaults()
.with(AlternativeHypothesis.LESS_THAN)
.with(PValueMethod.EXACT)
.with(Inequality.STRICT)
.test(x, y){code}
Effectively this renames the 'instance' method to 'withDefaults' to be
self-documenting.
This can be updated to the builder API if required after review. Such a change
would modify the public API and classes but the changes to the tests would be
minor, and it is refactoring of the tests that takes the most time.
> Port o.a.c.math.stat.inference to a commons-statistics-inference module
> -----------------------------------------------------------------------
>
> Key: STATISTICS-62
> URL: https://issues.apache.org/jira/browse/STATISTICS-62
> Project: Commons Statistics
> Issue Type: New Feature
> Components: inference
> Affects Versions: 1.0
> Reporter: Alex Herbert
> Priority: Major
>
> The o.a.c.math4.legacy.stat.inference package contains:
>
> {noformat}
> AlternativeHypothesis.java
> BinomialTest.java
> ChiSquareTest.java
> GTest.java
> InferenceTestUtils.java
> KolmogorovSmirnovTest.java
> MannWhitneyUTest.java
> OneWayAnova.java
> TTest.java
> WilcoxonSignedRankTest.java{noformat}
> The are few dependencies on other math packages. The notable exceptions are:
>
> 1. KolmogorovSmirnovTest which requires matrix support. This is for
> multiplication of a square matrix to support a matrix power function. This
> uses a double matrix and the same code is duplicated for a BigFraction
> matrix. Such code can be ported internally to support only the required
> functions. It can also drop the defensive copy strategy used by Commons Math
> in matrices to allow multiply in-place where appropriate for performance
> gains.
> 2. OneWayAnova which collates the sum, sum of squares and count using
> SummaryStatistics. This can be done using an internal class. It is possible
> to call the test method using already computed SummaryStatistics. The method
> that does this using the SummaryStatistics as part of the API can be dropped,
> or supported using an interface that returns: getSum, getSumOfSquares, getN.
> All the inference Test classes have instance methods but no state. The
> InferenceTestUtils is a static class that holds references to a singleton for
> each class and provides static methods to pass through the underlying
> instances.
> I suggest changing the test classes to have only static methods and dropping
> InferenceTestUtils.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
