Søren Hauberg wrote:
> I commited your patch; thanks! I made some small changes to fix some 
> compiler warnings. Could you check that they are okay? In 
> 'cl2bp_lib.cc:cl2bp' I initialised 
> 
>   imin = BIG_NUMBER 
>   k1 = k2 = -1 
 > is that okay?

Looks good.

> I also added extra parenthesis in line 80 (in 
> 'local_max'). Are they okay?

The "&&" operator has a higher precedence than "||", so your parentheses 
don't alter the semantics.  Since many people haven't memorized this 
table of like 15 different precedence levels, it definitely improves 
readability.

Of course, there are a lot of other obvious cleanups that could be made. 
  I hope to do that eventually, but actually the code has already come 
pretty far.  For programmers, complexity is the enemy, but for 
mathematicians I guess it's a central feature of the job.  ;-)

I also noticed that you commented out the L2error() function.  This 
function is required by the CL2BP_LOGGING switch, so I would suggest an 
#ifdef instead of a comment.  (But I question the premise of 
-Wunused-function, that these functions somehow imply a problem.)

One other note:  It would have been easier for me to analyze your 
changes if you had applied them as a separate SVN revision.  Just a 
suggestion.

Cheers,
-Pete

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
_______________________________________________
Octave-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/octave-dev

Reply via email to