[racket] Looking for feedback on code style

On Sep  9, David Van Horn wrote:
> On 9/9/10 11:26 AM, Eli Barzilay wrote:
> > On Sep  9, David Van Horn wrote:
> >>
> >> [...] As for the structure of the code as given, I would use helper
> >> functions in place of the `let'.  The resulting code will be easier
> >> to read and the helper functions can be tested independently, making
> >> it easier to maintain and improve the likelihood that the whole
> >> program is correct; and correctness and ease of maintenance are
> >> important aspects of style.
> >
> > Sounds like an overkill in this case, and for most values of
> > "idiomatic" I'd say that it'd make it less so.
> 
> This is what I had in mind.  I find this solution easier to read and
> reason about.  Is it really overkill?

IMO, for the original purpose that Scott originally mentioned
(teaching his son about medians), it's an overkill.  YMMV.

> Less idiomatic?

He asked about "idiomatic use of `let'" -- and if I view this as "in
the context of Racket/Scheme/Lisp", then splitting the `let' into a
separate function is not idiomatic.  (It might be if you take it as
"in the context of HtDP/intro-to-programming", but I didn't get that
meaning.)

Specifically, I wouldn't have split the code into a separate function
because:

* At the practical level, it doesn't buy me anything besides an extra
  `median-sorted' label, which I can get by a more descriptive
  comment:

    (define (median lon)
      (let ([sorted (sort lon)]
            [len    (length lon)])
        ;; we now find the middle one or two items in `sorted'
        ...same...))

  I find this easier to read -- for example, it's clear that `sorted'
  is sorted, and it's clear that it's a list of numbers since that
  context is two lines up.

  (For this reason I almost always prefer `let' for small
  one-call-site helpers, and will use a(n internal) function only if
  the helper code is more substantial.)

* You could argue that it introduces another point for testing -- but
  I find it much more natural to provide tests at the interface level.
  (And indeed you didn't write tests for `median-sorted' or `mean'.)

* The preconditions for the "...same.." part of the code are more
  easily enforced -- in this case, you need to specify that the list
  is sorted and that `n' is its length, and hope that you didn't do
  any mistakes in using it.  Translating this to a contract would make
  it larger than the code itself, and impractical for real code since
  the test will be very expensive.  Translating this to a type
  specification is hopeless for most type systems, including TR.
  OTOH, using a `let', the two constraints are obviously true.
  (Unless I have bugs -- but that's what the tests are for.)

* You're using another `mean' helper at the toplevel, and for a
  practical piece of code this means grabbing a precious piece of
  real-estate (my module's toplevel namespace).  (That's especially
  bad for some of us foreign-language-speakers that find "median"
  dangerously confusing when it stands next to "mean".)

But again, YMMV.

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!