From: Steve Smith
Subject: Request for criticism: Torrent file parser
Date: 
Message-ID: <87ll8thb4t.fsf@localhost.localdomain>
Hi,

As an simple exercise to start learning lisp I've been writing a
torrent-file parser/checker.  I now have something that works for the
common cases, and I'd like to request comments and suggestions from
the wizards.  The work so far is available here:

   http://people.vislab.usyd.edu.au/~ssmith/torrent-cl.tar.gz

If you prefer there's a Darcs repository here:

  http://people.vislab.usyd.edu.au/~ssmith/torrent/

Obviously this is a work in progress so not all functionality is
present, but any constructive criticism would be greatly appreciated.

Thanks,
Steve

From: Kim Minh Kaplan
Subject: Re: Request for criticism: Torrent file parser
Date: 
Message-ID: <1110807837.841015.231910@f14g2000cwb.googlegroups.com>
Note that I a no Lisp wizard, but here are some syntactic changes I
would apply to your code.

  * Use Common Lisp's DIGIT-CHAR-P instead of custom DIGITP
  * Don't assign result of READ-CHAR where it is not needed.
  * A Common Lisp STRING *is* an ARRAY.  No need to COERCE.
  * TO-SEQ is called COERCE in Common Lisp

  * Use extended LOOP instead of simple LOOP

      ;; Keep snarfing elements onto the list until the terminating
'e'.
      ;; Warning, mutual-recursive with bdec-next.
      (loop until (char= #\e (peek-char nil stream))
            collecting (bdec-next stream)
            finally (read-char stream)))

  * Use an adjustable STRING instead of COERCEing.

      (loop with str = (make-array 0 :element-type 'character
:adjustable t :fill-pointer 0)
            for char = (read-char stream)
            until (char= char delim)
            do (vector-push-extend char str)
            finally (return str)))


  * NORMALISE-FILE is best implemented using REDUCE.

    (reduce (lambda (a b) (concatenate 'string a "/" b))
            (gethash "path" hash)
            :initial-value basename)

  * TOTALLEN is best computed using REDUCE

    (reduce '+ files :key 'file-meta-length :initial-value 0)

Then I notice that you read the file as ASCII but I think you should
read it as bytes, especially since it contains binary data.

Also you swing your data back and forth from array to list, with some
reversing here and there.  You should stick to one form (array IMHO).

Kim Minh.
From: Matthieu Villeneuve
Subject: Re: Request for criticism: Torrent file parser
Date: 
Message-ID: <4236a9b0$0$17292$636a15ce@news.free.fr>
"Kim Minh Kaplan" <········@gmail.com> wrote in message
·····························@f14g2000cwb.googlegroups.com...
>   * Use an adjustable STRING instead of COERCEing.
>
>       (loop with str = (make-array 0 :element-type 'character
>                                      :adjustable t :fill-pointer 0)
>             for char = (read-char stream)
>             until (char= char delim)
>             do (vector-push-extend char str)
>             finally (return str)))

WITH-OUTPUT-TO-STRING seems more idiomatic to me here.

>   * NORMALISE-FILE is best implemented using REDUCE.
>
>     (reduce (lambda (a b) (concatenate 'string a "/" b))
>             (gethash "path" hash)
>             :initial-value basename)

This is better than the original code, but still conses a lot.
Using WITH-OUTPUT-TO-STRING would be a better solution, and using
FORMAT even better:

  (format "~A~{/~A~}" basename (gethash "path" hash))


--
Matthieu Villeneuve