Path: gmd.de!newsserver.jvnc.net!yale.edu!yale!gumby!wupost!zaphod.mps.ohio-state.edu!pacific.mps.ohio-state.edu!linac!mp.cs.niu.edu!news.ecn.bgu.edu!anaxagoras.ils.nwu.edu!riesbeck
From: ries...@ils.nwu.edu (Chris Riesbeck)
Newsgroups: comp.lang.lisp
Subject: Re: Loop macro
Date: 1 Apr 1993 17:37:56 GMT
Organization: The Institute for the Learning Sciences
Lines: 149
Distribution: world
Message-ID: <1pf99k$78o@anaxagoras.ils.nwu.edu>
References: <C4p52G.13K@stl.dk>
NNTP-Posting-Host: lyonesse.ils.nwu.edu


In article <C4p52...@stl.dk>, k...@stl.dk (Kjeld Larsen) writes:
> 
> To conclude the discussion of the loop-macro let us present
> the following piece of code written by John Burger and found 
> in the group comp.lang.clos:
>
> ... [original code deleted] ...
>
> The code segment reveals that the programmer masters mapping and
> lambdas, but it is not quite clear why that coding style is mixed
> with the loop-style. Below is 'the same' code, slightly modified for
> Lisp-style (sorry):
> 
> (defun least-common-superclass (instances)
>   (let ((candidates (reduce #'intersection
>                       (mapcar #'(lambda (instance)
>                                   (clos:class-precedence-list 
>                                     (class-of instance)))
>                               instances)))
>         (best-candidate (find-class t)))
> 
>     (mapl #'(lambda (candidates)
>               (let ((current-candidate (first candidates))
>                     (remaining-candidates (rest candidates)))
>                 (when (and (subtypep current-candidate best-candidate)
>                            (every #'(lambda (remaining-candidate)
>                                       (subtypep current-candidate 
>                                                 remaining-candidate))
>                                   remaining-candidates))
>                       (setf best-candidate current-candidate))))
>           candidates)
> 
>     best-candidate))
> 
> (Hope it's the same, it hasn't been tested)
>
> ... [text on Lisp programming style]... 
>
> - Kjeld & Flemming

As far as I'm concerned, both versions of this function
miss the boat.  Simply replacing LOOP FOR ... ON with MAPL 
doesn't really make a significant improvement in 
readability.

First, here's my version, then my philosophy of coding:

    (defun least-common-superclass (instances)
      (reduce #'more-specific-class
	      (common-superclasses instances)
	      :initial-value (find-class 't)))

    (defun common-superclasses (instances)
      (reduce #'intersection
	      (superclass-lists instances)))

    (defun superclass-lists (instances)
      (loop for instance in instances
	    collect (ccl:class-precedence-list
		     (class-of instance))))

    (defun more-specific-class (class1 class2)
      (if (subtypep class2 class1) class2 class1))


If you don't find this code significantly more readable,
skip to the next article.

The critical difference is not LOOP or mapping functions,
it's following some simple rules of coding.  My primary
rule for readable code is this:

	One function to a function.

That is, one function does one job. This usually means that
each function has one control structure, e.g.,
loop, dispatch, combine, etc.  The "slots" of the
control structure are filled in with simple calls to
other functions.  The names of those functions
document what's happening in a way that anonymous
chunks of code never can.

While you may fear an explosion of useless subfunctions, e.g., 

	(defun first-of-list (l) (first l))

most real code has the opposite problem of not enough
subfunctions.  Everyone wants to cram everything into one
package.

The "one function to a function" rule is intentionally
extreme, and owes a great deal to Strunk and White.  Like
a strict diet, you have to force yourself to stick with
it for the first few weeks. IMHO it's worth it.

Once your functions are down to one control structure
apiece, and that control structure involves repetition,
it often doesn't matter what looping form you pick.

I used LOOP in SUPERCLASS-LISTS, but I could have used
MAPCAR.  When functions are this simple, you can pick
what you like, or what you think is most efficient, and
it will be just as readable.  That goes for mapping, LOOP,
DO, series, etc.

Sometimes, it does matter.  There are limitations
in each kind of looping structure, and it doesn't make
sense to insist on only one kind.

LOOP ... COLLECT vs. MAPCAR -- LAMBDA is a tie for me,
but if you want to collect only certain values, then

	(loop for x in l
	      when <test x>
	      collect x)

is hands-down clearer than

	(mapcan #'(lambda (x) 
		    (when <test x> (list x)))
		l)

while 

	(remove-if-not #'(lambda (x) <test x>) l)

is OK but that double-negative leaves me cold.

On the other hand, when REDUCE is relevant, as in the
above example, it beats the corresponding LOOP FOR = THEN.
Too bad for LOOP supporters that it doesn't have something
like

	(loop for class-list in (superclass-lists instances)
              collect class-list using #'intersection


EXECUTIVE SUMMARY:

Good Lisp coding is like good nutrition:

     Eat light -- One function to a function.
     Eat a variety of foods -- Don't insist on just
	one iteration form.

Chris
-----