Re: [Changeset] new bzip2 function

[Thorsten Meyer]

John W. Eaton wrote:
> On  1-Jan-2009, Thorsten Meyer wrote:
>
> | John W. Eaton wrote:
> | > +      error (sprintf("%s: extension has to be a string with finite 
> length",
> | > +                     commandname));
> | >
> | > There's no need to use sprintf here since error already understands
> | > format strings.  So you should write
> | >
> | >   error ("%s: extension has to be a string with finite length", 
> commandname);
> | >   
> | Thanks for the hint.
> | 
> | Attached, you will find a little patch which fixes the error messages in
> | __xzip__ and adds a few tests of the error handling.
>
> | diff -r 628b6973b9fe -r 8f1edcf346b6 scripts/miscellaneous/__xzip__.m
> | --- a/scripts/miscellaneous/__xzip__.m      Mon Dec 29 11:21:42 2008 +0100
> | +++ b/scripts/miscellaneous/__xzip__.m      Thu Jan 01 11:24:18 2009 +0100
> | @@ -34,8 +34,7 @@
> |  
> |    if (nargin == 4 || nargin == 5)
> |      if (! ischar (extension) || length (extension) == 0)
> | -      error (sprintf("%s: extension has to be a string with finite length",
> | -                     commandname));
> | +      error ("__xzip__: extension has to be a string with finite length");
>
> Is it possible for these messages to be triggered by calling the bzip2
> or gzip functions incorrectly?  If so, then the __xzip__ in the
> messages won't help users much, and you should probably use the name
> of the calling function in the error message.
>   
First, I had the error messages like you propose. Then I thought it
would be quite confusing to have an error message that says "gzip: some
error" followed by the error trace that gives "__xzip__ at line xx" and
then "gzip at line yy".  Thinking about it again, it is probably even
more confusing to get an error message that says "__xzip__: expecting
argument to be this and that"  when you actually have to change the
argument of the calling function.

Here is a list of other error messages in internal functions that do not
reference back to the caller (and do not explain, what the problem is on
the caller level) or do not use the "caller: " style:

scripts/general/__isequal__.m:    error ("__isequal__: list objects are
deprecated and cannot be tested for equality here; use cell arrays
instead");
scripts/general/__splinen__.m:    error ("Incorrect number of arguments");
scripts/general/__splinen__.m:    error ("%s: non gridded data or
dimensions inconsistent", f);
scripts/image/__img__.m:    error ("__img__: matrix is empty");
scripts/image/__img_via_file__.m:    error ("the image viewing command
failed");
scripts/plot/__add_datasource__.m:    error ("internal error");
scripts/plot/__errcomm__.m:    error ("wrong argument types");
scripts/plot/__errcomm__.m:    error ("argument sizes do not match");
scripts/plot/__errcomm__.m:    error ("too many arguments to a plot");
scripts/plot/__errplot__.m:    error ("error plot requires 2, 3, 4 or 6
columns");
scripts/plot/__errplot__.m:    error ("error plot requires 2, 3, 4 or 6
columns");
scripts/plot/__gnuplot_ginput__.m:    error ("ginput: stream to gnuplot
not open");
scripts/plot/__go_draw_axes__.m:          error ("__go_draw_axes__:
invalid grid data");
scripts/plot/__go_draw_axes__.m:      error ("__go_draw_axes__: unknown
object class, %s",
scripts/plot/__go_draw_axes__.m:    error ("unsupported type of ticklabel");
scripts/plot/__go_draw_figure__.m:          error ("__go_draw_figure__:
unknown object class, %s",
scripts/plot/__go_draw_figure__.m:      error ("__go_draw_figure__:
expecting figure object, found `%s'",
scripts/plot/__next_line_color__.m:      error ("__next_line_color__:
color_rotation not initialized");
scripts/plot/__patch__.m:    error ("patch: not supported");
scripts/plot/__patch__.m:    error ("patch: color value not valid");
scripts/plot/__patch__.m:    error ("patch: size of x, y, and c must be
equal")
scripts/plot/__plr2__.m:      error ("__plr2__: invalid data for plotting");
scripts/plot/__plr2__.m:        error ("__plr2__: vector lengths must
match");
scripts/plot/__plr2__.m:        error ("__plr2__: vector and matrix
sizes must match");
scripts/plot/__plr2__.m:      error ("__plr2__: invalid data for plotting")
scripts/plot/__plr2__.m:        error ("__plr2__: vector and matrix
sizes must match");
scripts/plot/__plr2__.m:        error ("__plr2__: matrix dimensions must
match");
scripts/plot/__plr2__.m:      error ("__plr2__: invalid data for plotting")
scripts/plot/__plr2__.m:    error ("__plr2__: invalid data for plotting")
scripts/plot/__plt1__.m:    error ("__plt1__: options must be a struct
array");
scripts/plot/__plt2__.m:    error ("__plt1__: options must be a struct
array");
scripts/plot/__plt2__.m:      error ("__plt2__: invalid data for plotting");
scripts/plot/__plt2__.m:      error ("__plt2__: invalid data for plotting");
scripts/plot/__plt2__.m:      error ("__plt2__: invalid data for plotting");
scripts/plot/__plt2__.m:    error ("__plt2__: invalid data for plotting");
scripts/plot/__plt2mm__.m:      error ("__plt2mm__: arguments must be a
matrices");
scripts/plot/__plt2mm__.m:    error ("__plt2mm__: matrix dimensions must
match");
scripts/plot/__plt2mv__.m:    error ("__plt2mv__: matrix dimensions must
match");
scripts/plot/__plt2mv__.m:    error ("__plt2mv__: arguments must be a
matrices");
scripts/plot/__plt2ss__.m:    error ("__plt2ss__: arguments must be
scalars");
scripts/plot/__plt2sv__.m:    error ("__plt2sv__: first arg must be
scalar, second arg must be vector");
scripts/plot/__plt2vm__.m:    error ("__plt2vm__: matrix dimensions must
match");
scripts/plot/__plt2vm__.m:    error ("__plt2vm__: arguments must be a
matrices");
scripts/plot/__plt2vs__.m:    error ("__plt2vs__: first arg must be
vector, second arg must be scalar");
scripts/plot/__plt2vv__.m:    error ("__plt2vv__: vector lengths must
match");
scripts/plot/__plt__.m:      error ("plot: no data to plot");
scripts/plot/__pltopt__.m:      error ("__pltopt__: expecting argument
to be character string or cell array of character strings");

Should those also be fixed?
> Or, if these messages can only happen if __xzip__ itself is called
> improperly, then I suppose it is OK to leave them as they are, or it
> would also be OK to omit the checks entirely, since this is an
> internal function.
>   
The one check for correct call of __xzip__ itself is to prevent having
equal source and target file name which may lead to a loss of data in
some cases. I would rather leave that in.

Regards

Thorsten