Tue 13 Jul 2010 04:16:29 PM UTC, comment #1:
I took a look at the patch and I have three suggestions.
1) The octave coding guidelines call for a space between commas in function calls. rand(n,1) => rand(n, 1). This should be easy to fix.
2) In %test code blocks you don't need assign values to variables and then use assert to test whether those variables are equal. You can directly use assert on function calls.
%! rhoest = normest (A);
%! rho = norm (A);
%! assert (rhoest, rho, 1e-6);
could just be
%! assert(normest (A), norm (A), 1e-6);
3) The shape of the code looks like
A) Execute { loop 0 operations }
B) while (condition) { perform loop 1-N operations }
The loop code is duplicated. It appears outside the loop and inside the loop. If you want the loop to always execute once then, rather than using a while() control structure, you could try a do-until structure. I know that you just inherited the original code from normest, but it seems like it is worth fixing this at this time.
|