Scheme - Blackjack Program (SICP) / Learning to program

2
votes

I have decided to learn to program reading/doing SICP.

I'm using DrRacket and http://www.neilvandyke.org/racket-sicp/

I wrote a blackjack program https://github.com/fnava621/scheme_blackjack . It works.

Can you make this program more Readable and succinct?

Did I put too many comments? Not enough comments? How do I make this program "better"?

I'm also throwing down the gauntlet. Can you make a program that uses "optimal" strategy and determines what the probability of the player winning against the dealer (using n sample size)? I will do the same and compare code.

Thanks for reading/doing, Fernando Nava

2
schemeracketsicp
Perhaps you should try to write it in a harder scheme variant. Racket is quite different, easier. I personally don't like the 'if's. I can read and understand the whole program. – r4.

2 Answers

3
votes

A few suggestions:

  1. Avoid using so much mutation, ie set!.
  2. Avoid redefining built-in functions, like length.
  3. Add signatures and purpose statements to each of your functions. See here for an explanation.
  4. Write tests! Try using rackunit, documented here
2
votes

Let me focus on a single function show-deck. I see what it's doing, but the recursion can be simplified a bit. Making it easier to read might be slightly less efficient, but we're talking about 52 cards here... :)

The inner and outer loops are entangled in the original code. Here's one version that disentangles them:

(define (show-deck1 first-list second-list)
  (define (outer-loop first-list second-list)
    (cond
      ((null? first-list)
       '())
      (else
       (append (inner-loop (car first-list) second-list)
               (outer-loop (cdr first-list) second-list)))))

  (define (inner-loop x second-list)
    (cond
      ((null? second-list)
       '())
      (else
       (cons (cons x (car second-list))
             (inner-loop x (cdr second-list))))))

  (outer-loop first-list second-list))

We can apply a simplification: with the definition expressed this way, it's easier to see that map can be used to do the inner-loop.

(define (show-deck2 first-list second-list)
  (cond
    ((null? first-list)
     '())
    (else
     (define x (car first-list))
     (append (map (lambda (y) (cons x y)) second-list)
             (show-deck2 (cdr first-list) second-list)))))

This makes it easier to see the structure of the outer iteration. We can take map one step further, and use it for both the inner and outer loopings, and use (apply append ...) to flatten the substructure introduced by the nested use of map:

(define (show-deck3 first-list second-list)
  (apply append 
         (map (lambda (x)
                (map (lambda (y) (cons x y)) second-list))
              first-list)))

Your version avoids the (apply append ...) stuff altogether by cleverly threading the computation, but it's at the expense of some readability. One way to avoid the (apply append ...) and still get the benefit of easily seeing the nested loop structure is to use a foldr approach rather than a map approach:

(define (foldr f acc l)
  (cond
    ((null? l) acc)
    (else 
     (f (car l)
        (foldr f acc (cdr l))))))

(define (show-deck first-list second-list) 
  (foldr (lambda (x acc)
           (foldr (lambda (y acc)
                    (cons (cons x y) acc))
                  acc
                  second-list))
         '()
         first-list))

This should match what your original code did before. It relegates the loopness to foldr, though, so that all those iter functions disappear into uses of foldr.

Personally, if I'm writing in full Racket, I'd just code it with for loops. :) It'd look something like this:

;; If we're allowed to use full Racket:
(define (show-deck first-list second-list)
  (for*/list ([x first-list]
              [y second-list])
    (cons x y)))