HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Scheme/Racket: idiomatic infix math evaluator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
schemeevaluatorinfixidiomaticmathracket

Problem

Inspired by xkcd and a couple of praising blog posts, I decided to try out Lisp. It seemed that the best-supported dialect was Racket, itself a variant of Scheme, so I went with that and wrote an infix mathematical expression parser to get familiar with the basic concepts.

That was a mind-bending experience considering I never did functional programming. Not having mutable variables (or not using them, anyways) and having to use recursion instead of loops was a little hard for my imperative mind.

Anyways, I came up with this. My goal was to parse infix mathematical expressions with correct precedence and parentheses. It supports additions, subtractions, multiplications and divisions. It works, but it's probably full of hints that I'm new at this, and I would like to know which parts could have been made more idiomatic.

`#lang racket

;; usage: (reduce (tokenize))

;; tokenizing function
(define (tokenize)
(define (-tokenize-operator first)
(cond
([equal? first #\+] (list + (read-char)))
([equal? first #\-] (list - (read-char)))
([equal? first #\] (list (read-char)))
([equal? first #\/] (list / (read-char)))))

(define (-tokenize-number first)
(define (--char->number char)
(let ([ascii-zero (char->integer #\0)])
(- (char->integer char) ascii-zero)))

(define (--read-number initial)
(let ([char (read-char)])
(if (char-numeric? char)
(--read-number (+ (--char->number char) (* initial 10)))
(list initial char))))

(if (char-numeric? first)
(--read-number (--char->number first))
'()))

(define (-tokenize-openparen first)
(if (equal? first #\()
(list #\( (read-char))
'()))

(define (-tokenize first endchar)
(if (equal? first endchar)
'()
(let ([operator (-tokenize-operator first)]
[number (-tokenize-number first)]
[openparen (-tokenize-openparen first)])
(cond
([p

Solution

Here are my review comments on the tokeniser:

-
It's unusual to prefix symbols with -. The indentation should be enough to tell you the "nesting level" of the inner functions, without having to put -s in front.

-
Your inner -tokenize could be hoisted into a named let, so that your tokenize function looked like this (I will make other changes to the function later, but for now I try to keep as close to your original code as possible):

(define (tokenize)
  ;; other internal definitions

  (let recur ([first (read-char)]
              [endchar #\newline])
    (if (equal? first endchar)
        '()
        (let ([operator (-tokenize-operator first)]
              [number (-tokenize-number first)]
              [openparen (-tokenize-openparen first)])
          (cond
            ([pair? operator] (cons (car operator) (recur (cadr operator) endchar)))
            ([pair? number] (cons (car number) (recur (cadr number) endchar)))
            ([pair? openparen] (list (recur (cadr openparen) #\))))
            (else (recur (read-char) #\newline)))))))


-
Character comparisons can use eqv? instead of equal?. Not really a biggie, but usually I reserve equal? for deep comparisons.

-
Use #f for false/null values, not '(). In Scheme, #f is the only false value there is; '() is a true value (like everything else). Though, instead of all those different tokenising helpers, and calling them all and then only using one of the return values, it would be nicer to test directly in the cond.

-
Instead of all the repeated (read-char)s, you might consider using a port stream instead. Create one using sequence->stream, then you can call stream-first and stream-rest on it.

With all that in mind, I'd write the tokeniser like so:

(define (tokenize [port (current-input-port)])
  (let recur ([str (sequence->stream (in-input-port-chars port))])
    (cond [(stream-empty? str) '()]
          [(char-numeric? (stream-first str))
           (let loop [(str str)
                      (digits '())]
             (if (and (not (stream-empty? str))
                      (char-numeric? (stream-first str)))
                 (loop (stream-rest str) (cons (stream-first str) digits))
                 (cons (string->number (list->string (reverse digits)))
                       (recur str))))]
          [(char-whitespace? (stream-first str))
           (recur (stream-rest str))]
          [else (cons (stream-first str) (recur (stream-rest str)))])))


(Comments on the evaluator to come later.)

Code Snippets

(define (tokenize)
  ;; other internal definitions

  (let recur ([first (read-char)]
              [endchar #\newline])
    (if (equal? first endchar)
        '()
        (let ([operator (-tokenize-operator first)]
              [number (-tokenize-number first)]
              [openparen (-tokenize-openparen first)])
          (cond
            ([pair? operator] (cons (car operator) (recur (cadr operator) endchar)))
            ([pair? number] (cons (car number) (recur (cadr number) endchar)))
            ([pair? openparen] (list (recur (cadr openparen) #\))))
            (else (recur (read-char) #\newline)))))))
(define (tokenize [port (current-input-port)])
  (let recur ([str (sequence->stream (in-input-port-chars port))])
    (cond [(stream-empty? str) '()]
          [(char-numeric? (stream-first str))
           (let loop [(str str)
                      (digits '())]
             (if (and (not (stream-empty? str))
                      (char-numeric? (stream-first str)))
                 (loop (stream-rest str) (cons (stream-first str) digits))
                 (cons (string->number (list->string (reverse digits)))
                       (recur str))))]
          [(char-whitespace? (stream-first str))
           (recur (stream-rest str))]
          [else (cons (stream-first str) (recur (stream-rest str)))])))

Context

StackExchange Code Review Q#25400, answer score: 2

Revisions (0)

No revisions yet.