[Octave-patch-tracker] [patch #8379] signal package: Implemented uencode function

[Mike Miller]

Update of patch #8379 (project octave):

                Priority:              5 - Normal => 3 - Low                
                  Status:                    None => Need Info              
             Assigned to:                    None => mtmiller               
                 Summary: Implemented uencode function in signal package. =>
signal package: Implemented uencode function

    _______________________________________________________

Follow-up Comment #2:

Hi Georgios, thanks for the patch. Since this function is intended for the
signal package, I'm going to retitle it as such, to help me organize.

First, since this is intended to be in the signal package, could you redo this
as a changeset on the signal repository? The Octave Forge packages are
maintained separately from Octave itself, see http://sf.net/p/octave/signal
for the appropriate hg repository. Make sure you add new functions to the
appropriate section of the INDEX file when working on Forge packages.

Would you be interested in working on the udecode function as part of this
same contribution? It would be nice to get both functions working together in
one changeset, if you're interested in taking that on.

The coding style looks mostly correct. Some suggestions:

* Please wrap the Texinfo docstrings at 80 columns or less
* No need for Author and Created comments, version control tells us all we
need to know
* The nargin check should be the very first check, otherwise range checking on
n could fail
* Error messages should use the argument name, e.g. error ("func: N is not an
integer")
* Rewrite without using a for loop?

The last suggestion is a very good idea to get used to the style of
programming with Octave and other environments where vectors and matrices are
the primary data type. You should be able to rewrite the quantization part of
this function without using a for loop and loop index variable. There are many
examples in Octave and the signal package to draw from, feel free to ask on
the mailing list if you need help.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8379>

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