Path: ...!weretis.net!feeder8.news.weretis.net!eternal-september.org!feeder3.eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: "B. Pym" <Nobody447095@here-nor-there.org>
Newsgroups: comp.lang.lisp
Subject: Re: Another code review perhaps?
Date: Mon, 5 Aug 2024 17:10:00 -0000 (UTC)
Organization: A noiseless patient Spider
Lines: 148
Message-ID: <v8r10n$tdfv$1@dont-email.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Injection-Date: Mon, 05 Aug 2024 19:10:00 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="4d0da7ed839683006bcf3d49bfd7bbbd";
	logging-data="964095"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX18I/cmVApy33rYE0GRUmvKv"
User-Agent: XanaNews/1.18.1.6
Cancel-Lock: sha1:/dkdr96W5ZFHBWSqY5iIybc5Oi4=
Bytes: 6252

> > This is my solution to Ex. 5 on p. 97 of Paul Graham's "ANSI Common
> > Lisp"
> >
> > <QUOTE>
> > Define iterative and recursive versions of a function that takes an
> > object x and a vector v, and returns a list of all the objects that
> > immediately precede x in v.
> >
> > > (precedes #\a "abracadabra")
> > (#\c #\d #\r)
> > </QUOTE>
> >
> > (I'll just ask about the iterative solution I developed.)
> >
> > ;;;;Ex. 5
> > (defun precedes (object vector)
> >   (let ((maximum-vector-index (- (length vector) 1))
> >         (return-list nil))
> >     (dotimes (vector-index maximum-vector-index return-list)
> >       (let ((test-vector-element (aref vector (+ vector-index 1)))
> >             (preceding-vector-element (aref vector vector-index)))
> >         (if (and (eql object test-vector-element)
> >                  (not (member preceding-vector-element return-list)))
> >           (push preceding-vector-element return-list))))))
> >
> > Do you think that the use of DOTIMES is better than DO in this case?
> DOTIMES is fine. My main commentt is, while it's good to use
> descriptive names, you don't want to go overboard. And you don't
> really need to pull the (- (length vector) 1) expression out since
> it's only used once--I think it's actually more clear to use it
> directly in place; seeing it in place in the DOTIMES I know what it's
> for immediately. (Also, that's one of the benefits of using DOTIMES
> compared to DO, is that it only evaluates the count-form once).
> Finally, you can use PUSHNEW to do the membership test for you.
> Anyway, here's how I'd modify your original to make it (IMO) a bit
> more clear but otherwise about the same. Note how I haven't
> abbreviated any names--they're all full words. But I don't think
> anything is lost by, for example, using a single word, `index' instead
> of `vector-index':
> 
>   (defun precedes (object vector)
>     (let ((results nil))
>       (dotimes (index (1- (length vector)) results)
>         (let ((current (aref vector (1+ index)))
>               (previous (aref vector index)))
>           (when (eql object current)
>             (pushnew previous results))))))
> 
> Now that I can sort of see what's going on, I see that `current' and
> `previous' are also only used once each so I think it'll further
> clarify things to inline the expressions. I'd also abbreviate the
> index variable--not because it's less typing but because I can tell at
> a glance that it's just a regular index variable. Matter of taste:
> 
>   (defun precedes (object vector)
>     (let ((results nil))
>       (dotimes (idx (1- (length vector)) results)
>         (when (eql object (aref vector (1+ idx)))
>           (pushnew (aref vector idx) results)))))
> 
> That we've got things boiled down a bit we can try writing the
> equivalent using DO. Because we can control the starting value of the
> index with a DO loop I switch to starting at 1 and looping up to the
> end of the vector. I always try to have my index variable actually
> looping over the indices I want to use--I always screw it up if the
> end test is anything more complicated than (= idx (length vector)).
> 
>   (defun precedes (object vector)
>     (do ((length (length vector))
>          (results nil)
>          (idx 1 (1+ idx)))
>         ((= idx length) results)
>       (when (eql object (aref vector idx))
>         (pushnew (aref vector (1- idx)) results))))
> 
> I don't think that's really any better. Maybe LOOP:
> 
>   (defun precedes (object vector)
>     (loop with results = nil
>         for idx from 1 below (length vector)
>         when (eql object (aref vector idx))
>         do (pushnew (aref vector (1- idx)) results)
>         finally (return results)))
> 
> About the same as the DOTIMES version. However I might opt for
> expressing the removal of duplicates more explicitly, by using
> DELETE-DUPLICATES (which also lets me use LOOP's collect mechanism):
> 
>   (defun precedes (object vector)
>     (delete-duplicates
>      (loop for idx from 1 below (length vector)
>          when (eql object (aref vector idx))
>          collect (aref vector (1- idx)))
> 
> Or for a fairly different way of looking at it, there's already a
> function to find the position of a given object in a sequence. Maybe
> we can use it:
> 
>   (defun precedes (object vector)
>     (delete-duplicates
>      (loop for start = 1 then (1+ pos)
>          for pos = (position object vector :start start)
>          while pos collect (aref vector (1- pos)))))
> 
> I don't know if this last one is really better in any way but it's
> worth considering that there are a bunch of built in functions for
> doing good stuff with sequences.

newLISP

(define str "abracadabra")

(union (map str (map -- (clean zero? (flat (ref-all "a" (explode str)))))))

("r" "c" "d")


Another way:

(unique (find-all "(.)a" str $1))

("r" "c" "d")


Another way:

(define (prec c str)
  (difference
    (map (fn (m n) (if (= n c) m nil))
      (explode str)
      (explode (1 str)))
    '(nil)))

(prec "a" str)

("r" "c" "d")


Another way:

(define (prec c str  result)
  (for (i 1 (-- (length str)))
    (if (= c (str i) (push (str (- i 1)) result))))
  (unique result))

(prec "a" str)

("r" "d" "c")