[Newbie] How to improve my terrible calculator?

Questions & Answers
1

I am slightly past the "beginning student language" and I am trying to implement a simple calculator with what I have learned. However, I feel the code can be shorter and simpler but I don't know how. What's a better way to write this code? What direction should I go for this kind of program? Any suggestions are much appreciated.

#lang racket
(require racket/match)
(require rackunit)

(define (op? x)
  (or (equal? x #\+)
      (equal? x #\-)))

(define (lex s)
  (define (aux s pos)
    (if (or (= 0 (string-length s))
            (= pos (string-length s)))
        (list (string->number (substring s 0 pos)))
        (let ([h (string-ref s pos)])
          (cond [(op? h)
                     (cons (string->number (substring s 0 pos))
                           (cons (string->symbol (string h))
                                 (aux (substring s (add1 pos)) 0)))]
                [(char-numeric? h) (aux s (+ pos 1))]))))
  (aux s 0))

(check-equal? (lex "1+1") '(1 + 1))
(check-equal? (lex "1+1-1") '(1 + 1 - 1))

(define (calc s)
  (cond
    [(= (length s) 1) (first s)]
    [else (match (second s)
            ['+ (+ (first s) (calc (cddr s)))]
            ['- (- (first s) (calc (cddr s)))])]))

(check-equal? (calc (lex "1+1")) 2)
(check-equal? (calc (lex "1+4+2-3")) 4)
2

Are you reading htdp?

You should try using the design recipe to write this program. Right now, your two functions are each performing several tasks at once and it's hard to tell what's going on. Following the design recipe and other guidelines that htdp recommends will make this program clearer, more organized, and easier to work with.

At the very least, you should add purpose statements and signatures to your functions so what they are doing is more clear. Also, it would be very beneficial to choose more descriptive variable names. What is h?

I highly recommend htdp for generally learning how to systematically design programs and solve problems.

If you really want to make a calculator that takes in a string and evaluates it, I think you should wait until you're using advanced student language. You'll have learned a lot more tricks by then and you'll be able to do this much more cleanly. Also, don't try to use the full racket language yet. There are a lot of things in there that are confusing and will distract you from what's most important right now. There are things that are too powerful that you'll end up mis-using and forming bad habits with.

For now, I'd drop lexing and just focus on calc. I'd also recommend using prefix arithmetic like racket, rather than infix arithmetic like you'd write in other languages or by hand. Trying to parse infix arithmetic is a huge headache and in my opinion is not the interesting part of a calculator. If you really want infix, I'd recommend forcing everything to have parentheses around it.

Right now, your calculator is doing the order of operations incorrectly. For example, (calc (lex "9-4-3")) should be 2, but your calculator returns 8 because your calculator is right-associative. This means it's like your calculator interprets '(9 - 4 - 3) as 9 - (4 - 3) when it should be (9 - 4) - 3. Like I said, parsing infix arithmetic is tricky. That's why I recommend forcing everything to have parentheses around it, so it'd be '((9 - 4) - 3) or if you did prefix arithmetic, '(- (- 9 4) 3).

If you want to add multiplication and other operators into the mix, this'll get even messier. You'll have to make sure your calculator interprets '(1 + 2 * 3 + 4) as (1 + (2 * 3)) + 4, which would be a pain. If you really want to do that, you should add a parsing step before calc that converts an s-expression into a custom data type and then calc should take that in. For example, you'd have an add struct that has two fields, one for each operand. You'd also have a similar sub struct for subtraction.

Lexing involves converting a string into a list of tokens. In this case, a token is a number or a symbol for an operator. Parsing involves converting a list of tokens into some structured representation, like those custom data types I was talking about. Evaluation involves taking that structured representation and reducing it to a single, simple value. In this case, a number. Right now, you have parsing implicitly happening in calc, which also evaluates. If you want to add more operators, your arithmetic language will get complicated enough where it'll be worth having a parser. However, if you just require everything to be parenthesized, a parser isn't really necessary and it'll be easy to add something like multiplication to your calculator.

3 Likes
3

Just for fun, I write a my version of calc:

#lang racket

(require rackunit)

(provide (contract-out
          [calc+- (-> string? number?)]
          ))

(define (calc+- str)
  (let loop ([chars (string->list str)]
             [current_op +]
             [last_char_list '()]
             [result 0])
    (if (not (null? chars))
        (cond
         [(char=? (car chars) #\+)
          (loop
           (cdr chars)
           +
           '()
           (current_op result (string->number (list->string (reverse last_char_list)))))]
         [(char=? (car chars) #\-)
          (loop
           (cdr chars)
           -
           '()
           (current_op result (string->number (list->string (reverse last_char_list)))))]
         [(and (>= (char->integer (car chars)) (char->integer #\0))
               (<= (char->integer (car chars)) (char->integer #\9)))
          (loop
           (cdr chars)
           current_op
           (cons (car chars) last_char_list)
           result)]
         [else
          (loop (cdr chars) current_op last_char_list result)])
        (if (null? last_char_list)
            result
            (current_op result (string->number (list->string (reverse last_char_list))))))))

(check-equal? (calc+- "1") 1)
(check-equal? (calc+- "1+1") 2)
(check-equal? (calc+- "1+2-3-4") -4)
(check-equal? (calc+- "1 + 2 -3 -4 + 6") 2)
(check-equal? (calc+- "11+2-30-4") -21)
(check-equal? (calc+- "11+2-30-4+") -21)
1 Like