a random procedure

Question

I'm trying to write some code, but there is a problem with my reply procedure. Ignore the procedures for the first two random-of-threes, but the problem is in the else when calling the pick-random procedure. Here's the code:

(define earlier-responses '()) 

(define (doctor-driver-loop name earlier-responses)
  (newline)
  (write '**)
  (let ((user-response (read)))
    (cond 
      ((equal? user-response '(goodbye))
         (write-line (list 'goodbye name))
         (write-line '(see you next week)))
      (else 
         (list user-response earlier-responses)                    
         (write-line (reply user-response earlier-responses))
         (doctor-driver-loop name earlier-responses)))))

(define (reply user-response earlier-responses)
  (cond 
    ((= (random-of-three) 0)
      (append (qualifier)
              (change-person user-response)))
    ((= (random-of-three) 1)
      (hedge))
    (else
      (append (write-line '(earlier you said that))
              (pick-random earlier-responses)))))

(define (random-of-three)
  (random 3))

(define (pick-random lst)
  (nth (+ 1 (random (length lst))) lst))

It highlights

            (random (length lst))) lst)) 

It throws this error:

  random: contract violation
  expected: (or/c (integer-in 1 4294967087) pseudo-random-generator?)
  given: 0

I'm not sure what this error means or how to fix it...

Answer 1

(define random-4-digits (foldr (λ (n result) (cons n (map (λ (y) (if (>= y n) (add1 y) y)) result))) '() (map random '(10 9 8 7))))

Answer 2

I think you know what time it is.

[dons code-review hat /]

Firstly, before we get to anything else, what version of Racket are you using?

I've got 5.2.1 here, so I may be a bit behind, but nth and write-line don't seem to be defined functions here. I think what you want is list-ref and write. Note that list-ref has a different argument order, and expects a zero-indexed reference into the list. In other words (list-ref (list 1 2 3) 0) => 1. You therefore probably want to define pick-random as

(define (pick-random lst)
  (list-ref lst (random (length lst))))

You also have a couple of undefined functions there which I assume you define elsewhere in your program (hedge, qualifier and change-person). I'll elide these in further comments.

---

As Óscar mentions, random takes an integer in the range of 1 to 4294967087. Which means that you want to account for that before passing a number to it. As a note, (random n) seems to return an integer between 0 and (- n 1) inclusive, so you don't need to add or subtract things yourself to get a random element out of a list.

(define (pick-random lst)
  (if (null? lst)
      lst
      (list-ref lst (random (length lst)))))

---

As user448810 mentions, you don't want to re-calculate random-of-three because that won't give you equal probabilities of each choice being made. Since you'll be pre-calculating it, you probably don't need the random-of-three function at all.

(define (reply user-response earlier-responses)
  (let ((rand (random 3)))
    (cond ((= rand 0)
           (append (qualifier)
                   (change-person user-response)))
          ((= rand 1)
           (hedge))
          (else
           (append (write '(earlier you said that))
                   (pick-random earlier-responses))))))

---

write doesn't actually return anything. It just prints something to the REPL, so the else clause of your reply function can't be what you mean unless write-line does something fundamentally different from write.

(else
 (append (write '(earlier you said that))
         (pick-random earlier-responses)))

You either want

(else
   (write '(earlier you said that))
   (write (pick-random earlier-responses)))

if you just want printed output, or

(else
   (let* ((res (list '(earlier you said that) (pick-random earlier-responses))))
     (write res)
     res))

if you meant to preserve the return value for future use somewhere.

---

Next, and this is probably the biggest bug I see as well as the easiest for Scheme newbies to make (so don't feel bad), you're not changing earlier-responses anywhere. The else clause in your doctor-driver-loop

(else (list user-response earlier-responses)                    
      (write (reply user-response earlier-responses))
      (doctor-driver-loop name earlier-responses))

creates a new list with your user-response as the head and earlier-responses as the tail, but you don't do anything with it. You also don't pass a different value into the next call to doctor-driver-loop. If you actually wanted to track input history, you would want that to look something like

(else (write (reply user-response earlier-responses))
      (doctor-driver-loop name (list user-response earlier-responses)))

---

But that wouldn't quite do what you seem to want it to either. The above would always pass a two-element list to the next iteration of your driver loop; the first element would be the most recent response and the second element would be earlier-responses.

> (list '(I need a perscription) '((how are you doing?) (hello))) 
((I need a perscription) ((how are you doing?) (hello)))

> (list '(are you even listening?) '((I need a perscription) ((how are you doing?) (hello))))
((are you even listening?) ((I need a perscription) ((how are you doing?) (hello))))

What you'd really want, if you want to be able to pick a random past response using pick-random, is a flat list of all responses. That means consing new responses rather than listing them.

> (cons '(I need a perscription) '((how are you doing?) (hello)))
((I need a perscription) (how are you doing?) (hello)))

> (cons '(are you even listening?) '((I need a perscription) (how are you doing?) (hello)))
((are you even listening?) (I need a perscription) (how are you doing?) (hello))

So your else clause should be

(else (write (reply user-response earlier-responses))
      (doctor-driver-loop name (cons user-response earlier-responses)))

Answer 3

(or/c (integer-in 1 4294967087) pseudo-random-generator?)

is an "or" contract - meaning, to be accepted by this contract, the value (an argument to random function) must be one of the following: either an integer between 1 and 4294967087, inclusive, or an object such that (pseudo-random-generator? object) returns #t.

Answer 4

Oscar correctly answers the question you asked, but you have a worse error that you don't yet realize: You should calculate the random number once before entry to the cond expression rather than recalculating it in each cond clause. With your method you will not choose each of the three possibilities with equal probability. The first choice will be chosen about 1/3 of the time, the second choice will be chosen about 1/3 of the remaining 2/3 of the time, or 2/9, and the third choice will be chosen the remaining 4/9 of the time. You probably want all three choices to occur with equal probability.

Answer 5

The error is simply stating that the lists' length is zero, and random expects a value between 1 and 4294967087. Pass a non-empty list when calling pick-random. This is what's happening:

(random 0)

=> random: contract violation expected: (or/c (integer-in 1 4294967087)
   pseudo-random-generator?) given: 0

Try defining earlier-responses with a non-empty list at the beginning.