octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

new function: textscan.m


From: John W. Eaton
Subject: new function: textscan.m
Date: Fri, 22 Oct 2010 20:53:29 -0400

On 23-Oct-2010, Ben Abbott wrote:

| I've made an attempt to implement the missing function textscan.m
| 
| If there are no suggestions for improvement, I'll commit.

+  if (nargin > 2 && isnumeric (varargin{1}))
+    N = varargin{1};

I think it would help to quickly understand what N is if you used
nlines or similar instead of N.  Also, we generally try to avoid
uppercase variable names in Octave.

+  if ((! strcmp (class (fid), "double") || fid < 0) && ! ischar (fid))
+    error ("textscan: first input argument must be a valid file id, or 
string.");
+  endif
+
+  if (! ischar (formatstr) && ! isempty (formatstr))
+    error ("textscan: second input must be a format specification.");
+  endif

Maybe I'm just slow, but I have a harder time understanding negative
conditions like the ones above.  Instead of checking the conditions
that lead to errors, I find it simpler to write and easier to
understand code later if I test the conditions for success instead.
For example, instead of the above, I would write something like

  if (isa (fid, "double") && fid > 0 || ischar (fid))
    if (ischar (formatstr) || isempty (formatstr))
      ## ... code to do the real work here ...
    else
      error ("textscan: second input must be a format specification");
    endif
  endif
  else
    error ("textscan: expecting first argument to be a file id or character 
string");
  endif

Is that condition on formatstr correct?  Is it OK for it to be empty
if it is not a character string?

Note also that isa is probably better than class+strcmp.  But what
happens if fid is a matrix?  Should we check for that?  Should we
maybe have a is_valid_file_id function?  Maybe that would also be
useful in other places too.

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]