Hi Dean,

I see a lot of confusion about 'let' and perhaps also 'setq'.

I don't know how to better explain it as it is already done in the function
references. So I just put a few comments here; please try to understand how
exactly these functions work!


>    (dm ln_completes> (Ln Ln_no)
>       (let (Ln Ln Res 0)

This "Ln Ln" is a no-op. The variable 'Ln' is already bound as a parameter and
thus is already local. Instead, just do

        (let Res 0


>          (if (gt0 (: first_ln_no))
>             (let Ln (pack " " Ln)))

This has no effect. You bind 'Ln' again, to the result of the 'pack', but this
value is discarded because the body of the 'let' is empty.


>          (if (<> (: new_buf) NIL)

You never need to compare to NIL! Instead do

           (if (: new_buf)


>             (=: buf (: new_buf))
>             (=: buf (: hdngs)))
>          (if (member Ln (: buf))
>             (prog
>                (if (gt0 (: first_ln_no))
>                   (let Res (: first_ln_no))
>                   (let Res Ln_no))

Again, two empty 'lets' which have no effect at all.


>                (reset> This)
>                (let Res Res))

Same


>             (prog #not a member
>                (=: new_buf (fltr_mtchng_hdng_rmndrs Ln))
>                (if (<> (: new_buf) NIL)

Again as above


>                   (if (=0 (: first_ln_no))
>                      (=: first_ln_no Ln_no))
>                   (reset> This))
>                (let Res 0)))))

No effect again, so the result is NIL (= the return value of empty 'let's).


> Here's the original "setq" method
>
>    (dm ln_completes> (Ln Ln_no)
>       (if (gt0 (: first_ln_no))
>          (setq Ln (pack " " Ln)))
>       (if (<> (: new_buf) NIL)
>          (=: buf (: new_buf))
>          (=: buf (: hdngs)))
>       (if (member Ln (: buf))
>          (prog
>             (if (gt0 (: first_ln_no))
>                (setq Res (: first_ln_no))
>                (setq Res Ln_no))
>             (reset> This)
>             (setq Res Res))
>          (prog #not a member
>             (=: new_buf (fltr_mtchng_hdng_rmndrs Ln))
>             (if (<> (: new_buf) NIL)
>                (if (=0 (: first_ln_no))
>                   (=: first_ln_no Ln_no))
>                (reset> This))
>             (setq Res 0))))

This has the same issues with the comparison with NIL. Besides this, it writes
to the free variable 'Res', which might destroy its value in other functions.

I don't know why you are so obsessed with 'setq' ;)

The only place where it is good is the line (setq Ln (pack " " Ln)). For the
rest all 'setq's can be simply omitted if you fix the conditional flow.

Try it! :)

If you e.g. want to return 0 at the end, why don't you just write 0 instead of
(setq Res 0), assigning the value to a variable?

♪♫ Alex
-- 
UNSUBSCRIBE: mailto:picolisp@software-lab.de?subject=Unsubscribe

Reply via email to