On Tue, 28 May 2013 16:04:32 -0000, t...@apache.org wrote:
Author: tn
Date: Tue May 28 16:04:32 2013
New Revision: 1486982
URL: http://svn.apache.org/r1486982
Log:
[MATH-851] Added method MathArrays.convolve, thanks to Clemens Novak
for the patch.
-1
A significant part of this small patch (code and doc) contains
formatting
mistakes and non-conventional notations, as was indicated in the
comments.
If someone took the time to review something, please take it into
account.
The unit test is also not correctly defined; we agreed that newer code
should test _one_ thing per test method.
Also, precondition checks should use the "expected" attribute of the
"@Test" annotation.
Efficiency-wise, I wonder about the condition nested inside the
double-loop:
if ((j > -1) && (j < N) ) {
yn = yn + x[j] * h[k];
}
As was also suggested in the comments, a "real" application (and/or a
discussion on the "dev" ML, preferably initiated by the OP) would have
been welcome to assess the pertinence of including this code in the
proposed form.
This is a perfect example of "someone, some day, might need this".
IMO, contributions should come from people who _actually_ use the
proposed
code; we should not accept them as a mere coding exercises that will
potentially increase the burden on regular contributors.
Gilles
Modified:
commons/proper/math/trunk/src/changes/changes.xml
commons/proper/math/trunk/src/main/java/org/apache/commons/math3/util/MathArrays.java
commons/proper/math/trunk/src/test/java/org/apache/commons/math3/util/MathArraysTest.java
[...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
