Re: reading job-specifier.scm

[Wensheng Xie]

Hi, Dale,

Thanks for the detailed rationale. This is very good.

Now I understand more about the code.

I will take note for this.

best regards,
wxie

在 2022-01-14星期五的 13:31 +0000，Dale Mellor写道：
> On Thu, 2022-01-13 at 21:46 +0800, Wensheng Xie wrote:
> > Hi,
> > 
> > when reading the code of mcron, for example, the file job-
> > specifier.scm, I noted that the following code
> > 
> > ```
> > (define* (range start end #:optional (step 1))
> >   "Produces a list of values from START up to (but not including)
> > END.
> > An
> > optional STEP may be supplied, and (if positive) only every step'th
> > value will
> > go into the list.  For example, (range 1 6 2) returns '(1 3 5)."
> >   (let ((step* (max step 1)))
> >     (unfold (λ (i) (>= i end))          ;predicate
> >             identity                    ;value
> >             (λ (i) (+ step* i))         ;next seed
> >             start)))                    ;seed
> > ```
> > 
> > I want to understand what is the advantage here. Why don't you
> > write it
> > like
> > 
> > ```
> > (define* (range start end #:optional (step 1))
> >   "Produces a list of values from START up to (but not including)
> > END.
> > An
> > optional STEP may be supplied, and (if positive) only every step'th
> > value will
> > go into the list.  For example, (range 1 6 2) returns '(1 3 5)."
> >   (let ((step* (max step 1)))
> >     (if (>= start end)
> >         '()
> >         (cons start (range (+ start step*) end step*))))
> > ```
> > 
> > best regards,
> > wxie
> 
> 
> 
> Hello Wensheng,
> 
>    the real 'problem' with your version, as with some of the earlier
> versions of the code as seen below, is that the loop you have created
> is
> not properly tail-recursive, which means that the final range call
> has to
> involve stack manipulation and function call/return processing.
> 
> 
> 
>    You must understand that mcron is a twenty year old program these
> days
> and has gone through some small paradigm shifts as Guile itself has
> evolved and become more capable over the years.  In fact, the whole
> code
> was unceremoniously ploughed-over in early 2016, and commit 5da002
> changed
> the procedure from
> 
> ```
> (define (range start end . step)
>   (let ((step (if (or (null? step)
>                       (<= (car step) 0))
>                   1
>                   (car step))))
>     (let loop ((start start))
>       (if (>= start end) '()
>           (cons start
>                 (loop (+ start step)))))))
> ```
> 
> (which has the tail-recursion problem), to
> 
> ```
> (define* (range start end #:optional (step 1))
>   "..."
>   (unfold (cut >= <> end) identity (cute + <> (max step 1)) start))
> ```
> 
> which is arguably cleaner, although I really don't think it is; I
> find it
> very hard to see at first glance how the logic of this works.  It
> does
> solve the problem that there are no recursive function calls, though
> does
> use a relatively large memory space instead.  This version uses many
> constructs which were not available when the previous version was
> written
> around 2003.
> 
>    The same committer changed the code again a short time later to
> the
> current version.  It is telling that he had to include comments to
> explain
> the call of the unfold function!
> 
> 
> 
>    Your version also has the unfortunate effect of unnecessarily
> calling
> the max function multiple times when it is not needed, and would be
> better
> written as
> 
> ```
> (define (range start end #:optional (step 1))
>   "..."
>   (let loop ((start start) (step (max step 1)))
>      (if (>= start end)
>         '()
>         (cons start (loop (+ start step) step)))))
> ```
> 
> But if I was writing it today I would probably go with
> 
> ```
> (define (range start end #:optional (step 1))
>    "..."
>    (let ((step (max step 1)))
>       (iota (/ (- end start) step) start step)))
> ```
> 
> 
> 
>    So you see it is all about time and fashion and the state of Guile
> itself.  Speed is not really needed here, so anything which works,
> works.
> 
>    I had expected that one of the other maintainer/contributors to
> the
> code base would have addressed this, but I hope I have been able to
> answer
> your question,
> 
> Best wishes,
> Dale
>

signature.asc
Description: This is a digitally signed message part