Account Options

  1. Sign in
The old Google Groups will be going away soon, but your browser is incompatible with the new version.
Google Groups Home
« Groups Home
Message from discussion Newbie asking for help
The group you are posting to is a Usenet group. Messages posted to this group will make your email address visible to anyone on the Internet.
Your reply message has not been sent.
Your post was successful
 
From:
To:
Cc:
Followup To:
Add Cc | Add Followup-to | Edit Subject
Subject:
Validation:
For verification purposes please type the characters you see in the picture below or the numbers you hear by clicking the accessibility icon. Listen and type the numbers you hear
 
Pierre R. Mai  
View profile  
 More options Jun 22 2000, 3:00 am
Newsgroups: comp.lang.lisp
From: p...@acm.org (Pierre R. Mai)
Date: 2000/06/22
Subject: Re: Newbie asking for help

The Glauber <theglau...@my-deja.com> writes:
> In article <8iti9v$d1...@nnrp1.deja.com>, glauber wrote:

> [...]
> > ;; count number of lines in file key.html
> > (with-open-file (ifile "key.html" :direction :input)
> >                 (setf nlines 0)
> >                 (loop
> >                   (if (read-line ifile nil)
> >                     (setf nlines (+ 1 nlines))
> >                     (return nlines))))

> > It doesn't work (it just hangs, so i think it's stuck in the loop). I
> [...]

> OK. Using TRACE, i found that read-line wasn't returning nil, but
> something like #<END-OF-FILE>. So i added the third optional parameter,
> the one that tells it what to return at end of file. This made me do
> the (IF) backwards:

I think you should report that as a bug to the implementors of CLISP.
The HyperSpec is quite clear in that the default value for eof-value
should be nil, and therefore (read-line stream nil) should always
return nil at the end of file.  At least that is my reading of the
standard.

BTW: I very often supply the eof-value explicitly, for reasons of
clarity.

> (with-open-file (ifile "key.html" :direction :input)
>                 (setf nlines 0)
>                 (loop
>                   (if (read-line ifile nil nil)
>                     (setf nlines (+ 1 nlines))
>                     (return (print nlines)))))

> So, i plod along, undaunted...
> I'm pretty sure this is an ugly program, and i appreciate suggestions.

OK, you asked for it ;)

- The most obviously mistake lies in your use of setf to establish a
  new variable.  In the absence of any binding, this will be treated
  as a reference to a global special variable, which you don't want.
  Use let to introduce local lexical variables.  Once such a binding
  exists, you can alter the value of the binding via setf & friends,
  if you want to.

- The variable names are distinctly unlispy:  Since CL (and Scheme)
  allows useful separator characters in symbols, and your environment
  relieves you of the burden of retyping long names, you are free to
  use long, self-descriptive names.

- Wrap your code fragment in a function definition, and you get a
  ready made line-counting function for free...

- While working with low-level looping constructs can be instructive,
  most programmers would use a higher-level looping construct to
  more clearly express the intent of the code.

- Don't print the result yourself, since the surrounding code or the
  top-level read-eval-print loop will already do this for you

- Use documentation strings to document your functions

Given all of the above, your function might be written like this

  (defun count-lines-in-file (filename)
    "Count the lines in the file indicated by `filename'."
    (with-open-file (stream filename :direction :input)
      (do ((line-count 0 (1+ line-count)))
          ((null (read-line stream nil nil)) line-count))))

Or, using the extended loop facility:

  (defun count-lines-in-file (filename)
    "Count the lines in the file indicated by `filename'."
    (with-open-file (stream filename :direction :input)
      (loop while (read-line stream nil nil)
            sum 1)))

Or, using simple loop, like you did:

  (defun count-lines-in-file (filename)
    "Count the lines in the file indicated by `filename'."
    (with-open-file (stream filename :direction :input)
      (let ((line-count 0))
        (loop
          (if (read-line stream nil nil)
              (incf line-count)
              (return line-count))))))

Beware that all of the code is untested, though ;)

Regs, Pierre.

--
Pierre Mai <p...@acm.org>         PGP and GPG keys at your nearest Keyserver
  "One smaller motivation which, in part, stems from altruism is Microsoft-
   bashing." [Microsoft memo, see http://www.opensource.org/halloween1.html]


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.