[Octave-bug-tracker] [bug #53205] [octave forge] (signal) buttord function 's' option

[Mike Miller]

Follow-up Comment #5, bug #53205 (project octave):

The work you have done so far to add this option to the buttord function looks
very good. There has been some previous work done to add the analog option to
*all* of the ord functions. Would you mind taking a look at bug #46444 to
compare the work done there? I need to review all of this work and your work
and come up with the best combined improvements.

Aside from merging your work with bug #46444 and basic functionality, some
more coding style improvements would be very welcome in the revision that you
have currently.

As a few examples

* use "if (numel (Wp) == 1)" instead of "iff ( numel(Wp) == 1 )"
* use "if (Wp(1) > Ws(1))" instead of "if Wp(1) > Ws(1)"
* keep print_usage at the top of the function
* don't use printf in the function, if you need to warn, use the warning
function

You can also vastly simplify the test cases that you wrote at the end. Instead
of making a loop over some test index, just write each as its own test block.
The test function itself will take care of showing the code and the specifics
of the test failure if necessary. There are plenty of examples of well written
tests in Octave and in the signal package.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/bugs/?53205>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/