[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!