c
...@bilbao.com (Cad Bilbao) wrote in message <
news:604dab8.0111220655.28c1fb26@posting.google.com>...
> (setf myList '())
> (with-open-file (f "c:/input.txt" :direction :input)
> (loop for line = (read-line f nil nil)
> while line
> do (
> (setf myList(append myList line)
> );end-do
> );end-while
> );end-loop
> );end-with
> Any suggestion? Than you very much
Four suggestions:
1. Almost half the lines in your program are comments which tell you
nothing that the editor will not immediately tell you too, except it
will not get it wrong if you change the program, whereas yhour
comments will likely rapidly become incorrect (in fact, they are
already incorrect). Concentrate on comments that tell you things that
you or the editor can't immediately work out from the code.
(with-open-file (f "c:/input.txt" :direction :input)
;; third arg NIL means no error on EOF
;; fourth NIL means return NIL in that case.
(loop for line = (read-line f nil nil)
while line
do (
(setf myList (append myList line)))))
2. You have misunderstood the syntax of LOOP: all the forms following
DO until another LOOP keyword are evaluated, not a single list of
forms.
(with-open-file (f "c:/input.txt" :direction :input)
;; third arg NIL means no error on EOF
;; fourth NIL means return NIL in that case.
(loop for line = (read-line f nil nil)
while line
do (setf myList (append myList line))))
3. Your algorithm is quadratic in file length, since APPEND is linear
in list length. Obsessives will also worry that APPEND repeatedly
copies the list, but
that is a small problem. Either build the list backwards and reverse
it, resulting in a linear algorithm, or use one of the
building-a-list-forwards tricks either via one of the COLLECT-style
macros or using LOOP's COLLECT clause:
(with-open-file (f "c:/input.txt" :direction :input)
;; third arg NIL means no error on EOF
;; fourth NIL means return NIL in that case.
(loop for line = (read-line f nil nil)
while line collect line into lines
finally (setf mylist lines)))
4. The whole (SETF ...) then read the file and assign is really
horrible. If this is not extracted from a function it's also
technically ill-defined. If you want to define a top-level variable
do this:
(defparameter *my-lines*
(with-open-file (f "c:/input.txt" :direction :input)
;; third arg NIL means no error on EOF
;; fourth NIL means return NIL in that case.
(loop for line = (read-line f nil nil)
while line collect line)))
If you want to write a function that returns the lines in the file do
this:
(defun read-lines-from-file (file)
;; Return a list of all the lines in FILE. FILE is either a string
or
;; a pathname
(with-open-file (in file :direction ':input)
;; no real line can be NIL, so we don't need to worry about
;; inventing a unique return value here
(loop for line = (read-line in nil nil)
while line collect line)))
(defparameter *my-lines* (read-lines-from-file "c:/input.txt"))
Note that this program is the same length in lines as yours, but
includes four lines of comments which are actually useful: two of them
describe the function, and two describe why the READ-LINE works OK
(when, say, READ would need more care), one blank line for
readability, and defines a function which can be used elsewhere. And
it works (unless I made a typo, it is untested code), and is
efficient.
--tim
Note for CLL people: I think this is a great use of LOOP. It's *so*
easy to see what is happening here:
loop for line = <get next line from file, NIL on EOF>
while line collect line
Of course it's not pure functional Lisp. But *so what*?