RE: is a piece of code in 'polyval' really needed ?

[Bård Skaflestad]

Sorry about the top posting and lack of line feeds.  I left work early and I'm 
only a web-based client...

Yes, the version below is much easier to read than the original check.  I 
suppose, given that the return value is identical if ISEMPTY(x) || ISEMPTY(p), 
the two conditions could be fused as

   if (isempty (x) || isempty (p))
     y = [];
     return;
   elseif (! isvector (p))
     # as before
   endif

but that's really a matter of personal opinion.  On the other hand, there is an 
issue if NARGOUT==2 in this case (early return).  I haven't checked, but I seem 
to recall that you will at least get a warning about unassigned return values.  
Maybe, to be bullet proof, the early return case could be implemented as (some 
variation of)

  if (isempty (x) || isempty (p))
    [y, dy] = deal([]);
    return;
  elseif ...


Sincerely,
Bård Skaflestad <address@hidden>
SINTEF ICT, Applied Mathematics


________________________________________
From: Ben Abbott address@hidden
Sent: 18 August 2011 20:49
To: Bård Skaflestad
Cc: help-octave Octave; Sergei Steshenko
Subject: Re: is a piece of code in 'polyval' really needed ?

On Aug 18, 2011, at 1:34 PM, Bård Skaflestad wrote:

> On Thu, 2011-08-18 at 18:49 +0200, Ben Abbott wrote:
>> On Aug 18, 2011, at 11:31 AM, Sergei Steshenko wrote:
>>
>>> Hello,
>>>
>>> looking at 'polyval' implementation in octave-3.4.2 I see this:
>>>
>>> <snip>
>>>
>>> Is this:
>>>
>>>     65   if (length (p) == 0)
>>>     66     y = p;
>>>     67     return;
>>>     68   endif
>>>
>>> really needed ? My point is that, if I understand correctly,
>>>
>>>     52   if (! (isvector (p) || isempty (p)))
>>>
>>> check won't let code on lines ##65 .. 68 to be executed.
>
> [ snip ]
>
>> You are correct. Lines 65-68 aren't needed.
>
> Actually, they are.  Reread the actual check.

Thanks for catching that. Perhaps the syntax checking should be made more clear?

 42 function [y, dy] = polyval (p, x, s = [], mu)
 43
 44   if (nargin < 2 || nargin > 4 || (nargout == 2 && nargin < 3))
 45     print_usage ();
 46   endif
 47
 48   if (isempty (x))
 49     y = [];
 50     return;
 51   elseif (isempty (p))
 52     y = p;
 53     return;
 54   elseif (! isvector (p))
 55     error ("polyval: first argument must be a vector");
 56   endif
 57
 58   if (nargin > 3)
 59     x = (x - mu(1)) / mu(2);
 60   endif

Ben