[e-lang] Comments on SICP in E code

[Kevin Reid]

My comments on the code at

   <http://www.codepoetics.com/wiki/index.php? 
title=Topics:SICP_in_other_languages:E:Chapter_2>

. I've also made the changes that I think are appropriate and don't  
guess are against the purpose of the code.

> def gcd(a, b) {
>    if (b == 0) {
>       return a
>    } else {
>       return gcd(b, a % b)
>    }
> }

This sort of code can also be written in more functional style:

def gcd(a, b) {
    return if (b == 0) {
              a
           } else {
              gcd(b, a % b)
           }
}

or

def gcd(a, b) {
    return if (b == 0) {
       a
    } else {
       gcd(b, a % b)
    }
}

or

def gcd := fn a, b { if (b == 0) {
                         a
                      } else {
                         gcd(b, a % b)
                      }
                    }

> def nil() { }

Since nil here is of interest as a value, and not particularly as a  
function, it should be:

def nil {}

> def list(xs) {
>    if (xs.size() == 0) {
>       return nil
>    } else if (xs.size() == 1) {
>       return cons(xs[0], nil)
>    } else {
>       return cons(xs[0], list(xs(1)))
>    }
> }

The second case is unnecessary. I would write this as:

def list(xs) {
    if (xs =~ []) {
       return nil
    } else {
       return cons(xs[0], list(xs(1)))
    }
}

> def islist(xs) {
>    try {
>       if (xs == nil) {
>          return true
>       } else if (xs[1] == nil) {
>          return true
>       } else {
>          return islist(cdr(xs))
>       }
>    } catch err {
>       return false
>    }
> }

Again, the second case is unnecessary.

Don't name an unused variable except for documentation; use the  
ignore pattern instead:

    } catch _ {

Also, this function would be better written with patterns:

def islist(xs) {
    return switch (xs) {
       match ==nil    { true }
       match [_, cdr] { islist(cdr) }
       match _        { false }
    }
}

Or list guards:

def islist(xs) {
    if (xs == nil) {
       return true
    } else {
       def xsList := List.coerce(xs, fn _ { return false })
       return islist(cdr(xs))
    }
}

Or write the whole thing as a recursive composed guard:

interface Nil {}
def nil implements Nil {}

def LList := Tuple[any, LList] | Nil

def islist(xs) {
    return xs =~ _ :LList

    # more explicit form:
    # List.coerce(xs, fn _ { return false }); return true
}

I have not made this change in case the use of explicit ifs is  
significant.

> def str(s) { return `$s` }

def str(x) { return E.toString(x) }

def str := E.toString

> def min(x, y) { return if (x <= y) { return x } else { return y } }

This can be written as x.min(y). I have not made this change.

>    def print_rat(x) {
>       println (str(numer(x)) + "/" + str(denom(x)))
>    }

Use the printer, not concatenation.

    def print_rat(x) {
       println(numer(x), "/", denom(x))
    }

I'm skipping over most of the rationals code.

>       def dispatchx(m) {
>          if (m == 0) {
>             return x
>          } else if (m == 1) {
>             return y
>          } else {
>             throw ("Argument not 0 or 1 -- CONS " + str(m))
>          }
>       }

       def dispatchx(m) {
          return switch (m) {
             match ==0 { x }
             match ==1 { y }
             match _ {
                throw(`Argument not 0 or 1 -- CONS $m`)
          }
       }

I have not made this change except for the throw; I assume given the  
definition of str(_) above that `` is acceptable.

Also, dispatchx could be written more idiomatically as an object with  
multiple methods; in fact, it would be an ordinary E object definition.

-- 
Kevin Reid                            <http://homepage.mac.com/kpreid/>