Idiomatic code

[hsmyers]

In every language there is an idiomatic way of expressing the more common patterns. One of the things I am looking for is a more elegant (not to mention more efficient) way of returning values from a function or portion of a function (very nearly the same thing...) Consider:

Code: Select all

(define (parse-move n g)
  (let (s (get-move n g))
    (if (= 2 (length s))
		(filter on-board?
        (begin
          (if (black-to-move? n)
            (let (lst (list (next-diagl s)(next-diagr s)(next-row s)))
              (if (= "6" (row s))
                (append lst (list (next-row (next-row s))))
                (eval 'lst)))
            (let (lst (list (prev-diagl s)(prev-diagr s)(prev-row s)))
              (if (= "4" (row s))
                (append lst (list (prev-row (prev-row s))))
                (eval 'lst))))))
      (eval nil))))

Note the three evals used simply to return the list I want. I would hope that there is a better way. Trouble is I don't know it! Obviously I could wrap everything in (catch and then use (throw but that seems a kludge.

So how do you handle this kind of thing?

[Jeff]

catch and throw *is* the idiomatic way :)

But rather than using eval, investigate letex. Think of it as adding the cl back-tick read-macro to newlisp.

[Lutz]

instead of:

Code: Select all

(if (= "4" (row s)) 
            (append lst (list (prev-row (prev-row s)))) 
            (eval 'lst))

; and

(if (...)
   (...)
   (eval nil))

just do:

Code: Select all

(if (= "4" (row s)) 
            (append lst (list (prev-row (prev-row s)))) 
            lst)

; and

(if (...)
   (...))

Evaluating a quoted expression, is just the unquoted expression by itself, and evaluating 'nil', will just give 'nil' again. An 'if' expression returns 'nil' automatically if you leave out the else part. So returning 'nil' never needs to be specified in an 'if' expression.

[cormullion]

If I was writing this code I'd make it shorter anyway. That's just me. For example, without knowing what this code is, and what it's being called with, a first pass is:

Code: Select all

(define (f1 s)
   (filter on-board? 
        (begin 
          (if (black-to-move? n) 
            (let (lst (list (next-diagl s)(next-diagr s)(next-row s))) 
              (if (= "6" (row s)) 
                (append lst (list (next-row (next-row s)))) 
                (eval 'lst))) 
            (let (lst (list (prev-diagl s)(prev-diagr s)(prev-row s))) 
              (if (= "4" (row s)) 
                (append lst (list (prev-row (prev-row s)))) 
                (eval 'lst)))))))

(define (parse-move n g) 
  (let (s (get-move n g)) 
    (if (= 2 (length s)) 
        (f1 s)
         nil)))

That's one gone. I eckon you could spin out some more code into smaller functions. I just feel that the multiple if's are like potholes in the road, waiting to derail you at a later date. Ah mixed metaphor, sorry - but you know what I mean?!

It is a personal thing, style.

[hsmyers]

Jeff: No matter how much I stare at letex (which I keep reading as LaTeX for some reason) I don't see what you are getting at.

Note that I did see that the (eval nil) in the posted code is not necessary!

[hsmyers]

cormullion: You are right your revision is much clearer and certainly less prone to error.

Lutz: I'd say that I knew that nil was returned as value of missing second clause except that I forgot ;)

[hsmyers]

Code with suggestions incorporated:

Code: Select all

(define (parse-move n g)
  (let (s (get-move n g))
    (if (= 2 (length s))
        (_parse-move s))))

(define (_parse-move s)
   (filter on-board?
     (catch
        (if (black-to-move? n)
          (let (lst (list (next-diagl s)(next-diagr s)(next-row s)))
            (if (= "6" (row s))
              (append lst (list (next-row (next-row s))))
              (throw lst)))
          (let (lst (list (prev-diagl s)(prev-diagr s)(prev-row s)))
            (if (= "4" (row s))
              (append lst (list (prev-row (prev-row s))))
              (throw lst)))))))

For those who were wondering, this code returns a list of squares one of which will be where (origin square) the parsed move came from. Start with pawns first because the algebraic for a pawn move (e4 f5 d3 etc.) is easiest to parse. At some point this will be compared with all possible pawn moves for the side in question. Whoops! Starting to babel--- thanks for the help...