Mike Gran <spk...@yahoo.com> writes:
> I've been looking over one of the solutions to a Guile 100 problem
> (a basic version of the `ls' command) at the link below. It is
> interesting stylistically because it works very hard at minimizing the
> scope of procedures. It uses a lot of procedures within procedures,
> like, for example, within a `let*' block.
>
> https://gist.github.com/spk121/5301264
Apart from stylistic issues, I spotted a few errors in the code.
(digits (lambda (n) (if (eq? n 0) 0 (1+ (ceiling (log10 n))))))
This has two problems. First, the use of inexact arithmetic (via log10)
means that 'digits' is not guaranteed to compute the correct answer.
That's not good. Also, for purposes of this formatting task, (digits 0)
should be 1, not 0.
Second, 'eq?' is not guaranteed to work on numbers. '=' should be used
here, or 'zero?'. 'eqv?' could be safely used, but it has a different
meaning than '='. 'eqv?' tests "operational equivalence", whereas '='
tests "numerical equality". Numerical equality is the right thing here.
There are other places where 'eq?' is used inappropriately:
(bits-set?
(lambda (bits . masks)
(let ((mask (fold logior 0 masks)))
(eq? mask (logand bits mask)))))
The code also uses 'eq?' to compare characters, e.g.:
(define (string-starts-with? s c) (eq? c (string-ref s 0)))
(if (eq? type #\l) ...)
'eq?' is not guaranteed to work on characters either. To compare
characters, use 'char=?'. 'eqv?' could also work, but IMO it's cleaner
to use 'char=?' when you know that both arguments are characters.
Mark
