Cleaning up code through helper functions
We came up with the following code, that correctly computes the cost for an order
of pens, either with or without a slogan printed on them:
;; printed-pen-cost : number string -> number
;; consumes number of pens to order and a slogan and produces total order cost
(define (printed-pen-cost num-pens slogan)
(cond [(> (string-length slogan) 0)
(+ (* (+ (* 0.02 (string-length slogan)) (single-pen-cost num-pens)) num-pens) 3)]
[else (* (+ (* 0.02 (string-length slogan)) (single-pen-cost num-pens)) num-pens)]))
This code is rather ugly. Not to mention, it has the same somewhat
complicated computation twice, which gives us twice as much work to do
if we decide to change the formula for pen pricing. What should we do
about this?
Whenever you see the same computation multiple times in a program, you
should create a separate function for that computation and call that
function instead. This
- makes the program easier to read (because the function has a
descriptive name)
- lets you edit the program in only one place if something changes
To make the helper function, pull out the common code:
(* (+ (* 0.02 (string-length slogan)) (single-pen-cost num-pens)) num-pens)
Then wrap a function header (name and inputs) around the common code.
As inputs, you need whatever names are used in the common code that
aren't built-in or defined as constants.
(define (cost-without-surcharge num-pens slogan)
(* (+ (* 0.02 (string-length slogan)) (single-pen-cost num-pens)) num-pens))
Don't forget to add contract and purpose:
;; cost-without-surcharge : number string -> number
;; consumes number of pens and slogan and produces cost for pens with slogan, without the surcharge
(define (cost-without-surcharge num-pens slogan)
(* (+ (* 0.02 (string-length slogan)) (single-pen-cost num-pens)) num-pens)
Now, replace the common expression in the original code with a call to
your new helper function.
;; printed-pen-cost : number string -> number
;; consumes number of pens to order and a slogan and produces total order cost
(define (printed-pen-cost num-pens slogan)
(cond [(> (string-length slogan) 0) (+ (cost-without-surcharge num-pens slogan) 3)]
[else (cost-without-surcharge num-pens slogan)]))
Finally, let's define a constant for the price per character (again,
makes it easier to find if we need to change it later. The final
version looks like:
(define CHAR-COST .02)
;; cost-without-surcharge : number string -> number
;; consumes number of pens and slogan and produces cost for pens with slogan, without the surcharge
(define (cost-without-surcharge num-pens slogan)
(* (+ (* CHAR-COST (string-length slogan)) (single-pen-cost num-pens)) num-pens)
;; printed-pen-cost : number string -> number
;; consumes number of pens to order and a slogan and produces total order cost
(define (printed-pen-cost num-pens slogan)
(cond [(> (string-length slogan) 0) (+ (cost-without-surcharge num-pens slogan) 3)]
[else (cost-without-surcharge num-pens slogan)]))
This is the level of cleanliness and readability we expect in your code.