[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Octave-bug-tracker] [bug #53205] [octave forge] (signal) buttord functi
From: |
Mike Miller |
Subject: |
[Octave-bug-tracker] [bug #53205] [octave forge] (signal) buttord function 's' option |
Date: |
Tue, 20 Mar 2018 18:32:31 -0400 (EDT) |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0 |
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/
- [Octave-bug-tracker] [bug #53205] [octave forge] (signal) buttord function 's' option,
Mike Miller <=