SICP exercise 1.3 - sum of squares of two largest of three numbers

Question

From SICP

Exercise 1.3: Define a procedure that takes three numbers as arguments and returns the sum of the squares of the two larger numbers.

Square is:

(define (square x) (* x x))

I know the code could be written using language features like lambda or let, but first to remain true to the exercise, I wouldn't use features that are not discussed yet.

I took advantage of the fact that we only have three numbers that's why I am able to reduce my code into two if statements. All of the tests I've done produced correct output.

Please review my code.

(define (sum-of-squares a b c)
    (if (> a b)
        (+ (square a)
            (square (if (> b c)
                b
                c)))
        (+ (square b)
            (square (if (> a c)
                a
                c)))))

Here's an implementation with let. If possible I would like this reviewed as well.

(define (sum-of-squares a b c)
    (let ((bc (if (> b c) b c))
    (ac (if (> a c) a c)))
    (if (> a b)
        (+ (square a) (square bc))
        (+ (square b) (square ac)))))

Now, I find this second implementation not shorter than the first, but I think it's easier to read because it can be difficult to read expressions with more expressions nested inside as with the first implementation. On the other hand I realized it's worthless to do this because the variables aren't used more than once. For expressions more complicated than expressions contained in this procedure, would it be a good idea to put them inside let even if they are only used once?

How can I improve these codes and make them faster?

Answer 1

First of all, the name sum-of-squares is misleading. Longer names-separated-with-hyphens are quite acceptable in Scheme, so why not tell the whole truth? I suggest sum-of-squares-except-min or sum-of-squares-of-greatest-two.

The first solution is a bit of a brute-force approach. It works, but it is not obvious at a glance how it works unless you trace all paths of the decision tree.

The second solution, if the indentation and naming were improved, would be a bit clearer.

(define (sum-of-squares-except-min a b c)
    (let ((max-of-bc (if (> b c) b c))
          (max-of-ac (if (> a c) a c)))
         (if (> a b)
             (+ (square a) (square max-of-bc))
             (+ (square b) (square max-of-ac)))))

Personally, I would adapt the second approach and go "all the way" with it. Then, all the possibilities would be laid out logically, one per line.

(define (sum-of-squares-except-min a b c)
    (let ((least (min a b c)))
         (cond ((= least a) (+ (square b) (square c)))
               ((= least b) (+ (square a) (square c)))
               (else        (+ (square a) (square b))))))

min should be built in, but you can implement it as a helper function yourself if you need to.

Alternatively, sum all the squares, then subtract the square of the least.

Which approach is fastest? It's hard to say, without benchmarking, whether it's better to define fewer intermediate variables, do less branching, or do less arithmetic. I wouldn't worry about performance for such a simple problem.