On Tue, 28 May 2013 20:47:30 +0200, Thomas Neidhart wrote:
On 05/28/2013 08:20 PM, Gilles wrote:
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.

Most of the suggestions were already corrected by the OP with an updated patch, the rest I changed to the usual style I use when committing (see
the commit log).

My reply was based on reading the diff.

[Are you really using capital letters as variable names?]

Efficiency-wise, I wonder about the condition nested inside the
double-loop:

              if ((j > -1) && (j < N) ) {
                  yn = yn + x[j] * h[k];
              }

yes I agree, this could be further improved.

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.

It is also present in numpy (see

http://docs.scipy.org/doc/numpy/reference/generated/numpy.convolve.html)
so I guess there will be a practical use for it.

By this post, I ask whether it is sufficient reason for committing
sub-par code that is currently not used by anyone.
I suggested the OP 3 ways to start getting involved; the absence of
reaction was IMHO reason enough not to rush including this code.
[It has obviously nothing to do with judging that convolution is not
worth implementing.]

CM needs contributors, right; but it is quite unrelated to piling up
unused/untested code.
At one point, some years ago, there was a sort of acknowledgment that
submitting a contribution was accompanied with an informal commitment
to maintain it.
Lacking that either gives more work to current committers or lowers
the average quality of the library.


Regards,
Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to