Why won't my solution to SICP Exercise 1.3 work?

Question

I just began working through SICP and I'm doing the first problem set, namely Exercise 1.3: "Define a procedure that takes three numbers as arguments and returns the sum of the squares of the two larger numbers."

(define (toptwosq x y z)
  (cond ((and (> x y) (> z y))) (+ (* x x) (* z z))
        ((and (> y x) (> z x))) (+ (* y y) (* z z))
        ((and (> x z) (> y z))) (+ (* x x) (* y y))))

When I run this, I get pretty odd results(none of which get me the sum of the squares of the largest two numbers). I've found other solutions that work and I understand why they work...but why doesn't mine?

Answer 1

You're closing the cond clauses too early.

((and (> x y) (> z y))) is your first cond clause, which will return #t if true and #f otherwise, and if true will make the value of the cond to be #t.

(+ (* x x) (* z z)) is your second cond clause, which will always return the value of the sum of the square of x and the square of z, making the cond statement return that value as any value other than #f is considered and true. Sometimes it's useful to exploit this one-part clause, but most of the time you want to use two part clauses.

(define (toptwosq x y z)
  (cond ((and (> x y) (> z y)) (+ (* x x) (* z z)))
        ((and (> y x) (> z x)) (+ (* y y) (* z z)))
        ((and (> x z) (> y z)) (+ (* x x) (* y y)))))

and you really should have an else clause

(else (+ (square x) (square y)) 

As none of the cases you've put out so far will catch the case of x y and z being the same value.

Get an editor that does parenthesis matching and you life will become easier.

Answer 2

As @WorBlux pointed out, you have some parenthesis problems. Besides that, I have a couple of tips:

• It's a bit clearer if you use nested ifs to separate conditions
• Your conditions are not correct, the equality cases are missing
• If the conditions are right, it won't be necessary to have a catch-all else case
• You should declare a helper procedure for performing the actual squared sum

This is what I mean:

(define (sumsq x y)
  (+ (* x x) (* y y)))

(define (toptwosq a b c)
  (if (>= a b)
      (if (>= b c)
          (sumsq a b)
          (sumsq a c))
      (if (>= a c)
          (sumsq b a)
          (sumsq b c))))

The same code can be written as follows using cond, notice how to correctly express the conditions in such a way that all cases are covered:

(define (toptwosq a b c)
  (cond ((and (>= a b) (>= b c)) (sumsq a b))
        ((and (>= a b) (<  b c)) (sumsq a c))
        ((and (<  a b) (>= a c)) (sumsq b a))
        ((and (<  a b) (<  a c)) (sumsq b c))))

The last condition can be replaced with an else. It's not a "catch-all", we're certain that at this point no more cases remain to be considered:

(define (toptwosq a b c)
  (cond ((and (>= a b) (>= b c)) (sumsq a b))
        ((and (>= a b) (<  b c)) (sumsq a c))
        ((and (<  a b) (>= a c)) (sumsq b a))
        (else                    (sumsq b c))))

And finally, if we're smart we can get rid of one case (the first and third cases are the same) and simplify the conditions even more:

(define (toptwosq a b c)
  (cond ((or (>= a b c) (and (>= a c) (> b a)))
         (sumsq a b))
        ((and (>= a b) (> c b))
         (sumsq a c))
        (else (sumsq b c))))

Answer 3

Just on a tangent, this is how the solution code could be derived, from a higher order description.

With equational syntax, (read $ as "of"; f x signifies application, parenthesis used for grouping only),

sum_sqrs_of_biggest_two (a,b,c) =                -- three arguments

      = sumsqrs $ take 2 $ sort [a,b,c]          -- list of three values
      = sumsqrs $ take 2 $ merge (sort [a,b]) [c]
      = sumsqrs $ take 2 $ 
          if a >= b
            then merge [a,b] [c]
            else merge [b,a] [c]
      = sumsqrs $
          if a >= b 
            then if b >= c then [a,b] else [a,c]
            else if a >= c then [b,a] else [b,c]
      = if a >= b 
            then if b >= c then a^2+b^2 else a^2+c^2
            else if a >= c then a^2+b^2 else b^2+c^2

... and translate it back to the Scheme syntax.