Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ch. 5, Purely Relative: Median function doesn't return median #156

Open
mikmart opened this issue Feb 28, 2018 · 1 comment
Open

Ch. 5, Purely Relative: Median function doesn't return median #156

mikmart opened this issue Feb 28, 2018 · 1 comment

Comments

@mikmart
Copy link

mikmart commented Feb 28, 2018

In the Purely Relative section of Chapter 5 you define a function called median to use in an example, but it doesn't actually find the median: it's just returning the average of the first and last values in the array. (Which just coincidentally is also the median in the specific array used in the example.)

Actually defining median inline might be a bit too complicated (as the purpose of the function in the example is just to illustrate the side effects of modifying the array referenced by the closure) so maybe the function should just be renamed? Or alternatively it could be changed to calculate the mean, since that would give the same output in the example, and is simple to define?

@getify
Copy link
Owner

getify commented Feb 28, 2018

Thanks for bringing up this point.

IMO, this is a classic case of why sometimes using foo(..) / bar(..) type names is better than trying to come up with domain-specific names. The point of the section has nothing to do with what the utility does, only how it does it. Any domain-specific name invites the reader to be distracted by what it does. But, I agree I shouldn't have used a name like median(..), since that name implies it's a generalized utility.

I could have called it more accurately center(..) or middle(..), though those names only work if you know the list is at least partially sorted (the lowest number first, highest number last). I don't really want to complicate what the function does, since it isn't the point of the discussion. I may just change it to foo(..) here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants