How can I improve this auxilary function in Racket?

Question

I'm working in HtDP, Chapter 4 using the BSL language.

The problem I was working on is:

Exercise 136: If you run main, press the space bar (fire a shot), and wait for a good amount of time, the shot disappears from the canvas. When you shut down the world canvas, however, the result is a world that still contains this invisible shot.

Design an alternative tock function, which not just moves shots one pixel per clock tick but also eliminates those whose coordinates places them above the canvas. Hint: You may wish to consider the design of an auxiliary function for the recursive cond clause.

The solution that I came up with is below (in a spoiler). However, I feel that I'm doing something redundant. Basically my application of the auxiliary function isn't quite correct.

(define (main w0)
  (big-bang w0
            (on-tick ticking)
            (on-key fire-key)
            (to-draw to-render)))

(define HEIGHT 100)
(define WIDTH 80)
(define TURRET-X-POS (/ WIDTH 2))
(define BKGRND (empty-scene WIDTH HEIGHT))
(define SHOT-IMG (triangle 4 "solid" "red"))

(define (to-render w0)
  (cond
    [(empty? w0) BKGRND]
    [else (place-image SHOT-IMG TURRET-X-POS (first w0) (to-render (rest w0)))]))

(define (fire-key w0 ke)
  (cond
    [(key=? ke " ") (cons HEIGHT w0)]
    [else w0]))

(define (ticking w0)
  (cond
   [(empty? w0) empty]
   [(empty? (only-inbound-shots w0)) empty]
   [else (cons (sub1    (first (only-inbound-shots w0))) 
               (ticking (rest  (only-inbound-shots w0))))]))

(define (only-inbound-shots w0)
  (cond      
    [(< (first w0) -4) (rest w0)]
    [else w0]))

UPDATE:
(This is much cleaner than before)

(define HEIGHT 100) ;height of scene
(define WIDTH 80)   ;width of scene
(define TURRET-X-POS (/ WIDTH 2)) ;position of turret, ie. shot's x-coordinate
(define BKGRND (empty-scene WIDTH HEIGHT)) ; scene itself
(define SHOT-IMG (triangle 4 "solid" "red")) ;image representing the shot
(define Y-BOUNDARY -4) ;y-coordinate where shot is no longer visible in scene

;List-of-numbers -> List-of-numbers
;renders all shots fired
(define (to-render w0)
  (cond
    [(empty? w0) BKGRND]
    [else (place-image SHOT-IMG TURRET-X-POS (first w0) 
                       (to-render (rest w0)))]))

;List-of-numbers, key event -> List-of-numbers
;only allows the space bar to fire a shot
;one space bar event produces one shot
(define (fire-key w0 ke)
  (cond
    [(key=? ke " ") (cons HEIGHT w0)]
    [else w0]))

;List-of-numbers -> List-of-numbers
;during each clock tick, the y-coordinate each of the shot 
;                                      in List-of-numbers is updated
;each y-coordinate decreases by -1
(define (ticking w0)
  (cond
    [(empty? w0) w0]
    [else (only-inbound-shots (update-shots w0) Y-BOUNDARY)]))

;List-of-numbers -> List-of-numbers
;does the actual updating of the shots in List-of-numbers
;each shot's value is decreased by -1
(define (update-shots w0)
  (cond
    [(empty? w0) w0]
    [else (cons (sub1 (first w0)) (update-shots (rest w0)))]))

;List-of-numbers -> List-of-numbers
;checks to see if the first shot in the List-of-numbers has gone past the Y-BOUNDARY
;if so then remove shot from the List-of-numbers and return the rest of the List
;otherwise return the List without change
(define (only-inbound-shots w0 y-boundary)
  (cond
    [(empty? w0) w0]
    [(< (first w0) y-boundary) (rest w0)]
    [else w0]))

;List-of-numbers -> List-of-numbers
;creates the world of shots
;seed value is empty, additional values created by space bar
(define (main w0)
  (big-bang w0
            (on-tick ticking)
            (on-key fire-key)
            (to-draw to-render)))

TESTS added:
I'm still working on the tests.

(define test-shots
  (cons -6 (cons -5 (cons 10 empty))))

(define test-shots-2
  (cons -6 (cons 2 (cons 7 empty))))

(define test-shots-3
  (cons 4 (cons 9 (cons 10 empty))))

(check-expect (to-render test-shots) 
  (place-image SHOT-IMG TURRET-X-POS -6
    (place-image SHOT-IMG TURRET-X-POS -5
      (place-image SHOT-IMG TURRET-X-POS 10
        BKGRND))))


(check-expect (to-render test-shots-2) 
  (place-image SHOT-IMG TURRET-X-POS -6
    (place-image SHOT-IMG TURRET-X-POS 2
      (place-image SHOT-IMG TURRET-X-POS 7
        BKGRND))))

TEST with world functions added:

(define HEIGHT 1) ; makes test a little faster

(check-expect
  (fire-key 
    (ticking 
      (ticking 
        (ticking 
          (ticking 
            (fire-key 
              (ticking 
                (ticking 
                  (ticking 
                    (ticking (fire-key empty " "))))) 
            " "))))) 
    " ")
  (cons -3 (cons 1 empty))

Answer 1

• The usual comments about missing contracts, purpose statements, and data definitions apply here. As well as tests of the individual functions; a big reason why world.ss/universe.ss are really nice libraries is that they enable one to test functions that are conceptually performing Input/Output.

• I'm inferring a lot about what your data definition is from the code, but (1.) you should not put that onus on the reader, and (2.) it could lead to mistakes in my reasoning.

• It looks to me like you have deviated significantly from the template in your definition of ticking; it does not look like any template I can think of. A similar comment applies to only-inbound-shots

• You may want to break ticking up into multiple subroutines, and then compose them.

  An example of what I mean by this: If you were to make a function to take the average of a list of numbers, a simple way to do it is to make two new functions: the first produces the sum of the numbers, and the second produces the length of the list; these are trivial to write via the Design Recipe. Then average is:

  ;; average : [Listof Number] -> Number
  ;; produces average value of input (x_1 x_2 ... x_n
  (define (average l)
    (/ (sum-of-list l) (length-of-list l)))
  

  But if you were to try to do it in a single definition of average that followed the template for [Listof Number], you would have some problems getting the right answer. (I do not think it can be done properly without using an accumulator or two.)

  That factoring into very simple subroutines and then composing them at the end to get the desired effect is what I mean by breaking ticking up and then composing the pieces. (If you're not destructuring your input, function composition is a perfectly valid design process: see HtDP section 3.1.)

• More importantly, though, I think is to make some tests for the individual functions. Especially only-inbound-shots: I suggest you think about this function on its own.

  • Pretend that you don't know who might call it, and only that they will obey its contract (e.g. they will only pass in a World, whatever you defined that to be here).
  • And then make sure you produce the right answer for any possible legal input they provide.
  • Don't think about how you use it yourself in your other code above, because you don't want to try to keep all that in your head at the same time. Its actually simpler to generalize here, and think about what only-inbound-shots should do on any possible input.

To provide you with some concrete food for thought on the matter of testing, here are some hypothetical pictures describing the inputs you might try to handle in your tests:

, ,

---

Update 28 Feb 2013:

While I still recommend writing individual unit tests of each of your functions, end-to-end testing is also important. In this case, the game as currently rendered won't tell you if have shots lying outside the scene or not (because place-image, unlike say overlay, automatically crops them from the rendering).

So, if you want to debug the game while it is running, it can be useful to get that kind of information. Say like a drop down bit of text that renders on top of the game (one often sees this in video games to show you things like Frame Rate). So here is one strategy for getting that information out while the game is running: Swap in an alternative rendering function, that is layered on top of your existing one, but prints out other information about the world w0 argument.

(In this case, it might be useful to see its length, though one can imagine extracting other information.)

;; List-of-numbers -> Image
;; Renders w0 via to-render, with a printout of shot count in top left corner.
(define (to-render-with-count w0)
  (place-image/align (text (number->string (length w0)) 30 'blue)
                     0 0 "left" "top"
                     (to-render w0)))

Then you hook in to-render-with-count in your big-bang invocation. It may also be useful to slow down the clock tick rate, so that you can see what happens as keystrokes and clock ticks are intermixed, so I have made that change too (in the on-tick clause):

(define (main w0)
  (big-bang w0
            (on-tick ticking 0.1)
            (on-key fire-key)
            (to-draw to-render-with-count)))

Now, I can interactively notice interesting trends. Trends that yield situations like this:

How is it that I have 148 balls on the screen but only four are showing? What kind of world would have that happen? (If you close the window created by big-bang, it will return the current world to the Interactions Window, so you will see right there exactly what kind of World would have that happen.)