commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thomas Neidhart <thomas.neidh...@gmail.com>
Subject Re: [Math] Accepting contributions (Was: svn commit: r1486982 - ...)
Date Mon, 03 Jun 2013 19:02:20 GMT
On 05/28/2013 08:20 PM, Gilles wrote:
> On Tue, 28 May 2013 16:04:32 -0000, tn@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.

I improved the code / javadoc. Do you now agree with it?

Thomas

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


Mime
View raw message