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

largest-series-product: Should tests test for implementation details (slices, digits)? #192

Closed
petertseng opened this issue Mar 8, 2016 · 4 comments

Comments

@petertseng
Copy link
Member

I've mentioned in a previous comment, but I'm taking out just the relevant points here:

Some tracks' test suites for largest-series-product too severely funnel students toward a particular implementation whereas there are at least two implementations to choose between (1. generate all slices, find all products, choose the max. vs 2. iterate all digits once, keeping a sliding window and the maximum product seen). They test internal implementation details (some helper functions) instead of the actual problem (Finding the largest series product!). This means that for my Haskell and Clojure submissions for this problem I was "forced" to write a helper function that my actual submission doesn't use to find the largest product.

I propose that all tracks that implement largest-series-product remove tests for implementation details, and just leave tests for the actual problem. Note that this was previously done for the Ruby track in exercism/ruby#10, and that I deliberately chose not to include implementation details in the JSON shared test data at https://github.com/exercism/x-common/blob/master/largest-series-product.json#L7

(To make it clear, I'm fine with personally submitting PRs to execute this proposal myself, but I'd like to get consensus before I know that I should actually do that)

Playing devil's advocate against myself, there is a distinct advantage of writing test suites that guide students toward a solution: They give students hints on a potentially hard problem. This is actually a fine thing to do! But I wish there were a way of giving hints that didn't restrict students like this. Maybe I suggest how Elixir did it here? https://github.com/exercism/xelixir/tree/master/exercises/zipper/hints

Yet another advantage of writing test suites in this way: It's a good thing to get students in the habit of testing helper functions so that they can build larger solutions from smaller parts that they know to be working. But I'm going to argue here that this should be up to the student to do this! Any way to guide them in that direction? Recall that many tracks that implement largest-series-product also implement series (which is where it makes sense to test the slices and digits functions!).

Given that I think most largest-series-product appear late in their respective tracks (at a stage where students should know how to test internal details themselves, plus anyone who hasn't been skipping around has already written series if applicable for their track), I think this is reasonable to do, but I'd like to hear anything I missed.


Appendix:

The list of tracks with largest-series-product:
http://x.exercism.io/problems/largest-series-product

With series:
http://x.exercism.io/problems/series

So, languages that have LSP but not series: clojure, elixir

I put links to the largest-series-product subdirectory for each track in https://github.com/exercism/todo/issues/104#issuecomment-174168732

I believe the languages that test digits/slices are:
clojure, csharp, ecmascript, elixir, haskell, javascript, perl5, python, scala

(I also made a mistake with csharp - the case for empty string and 0 span should be a product test, not a slice test)

@petertseng petertseng changed the title largest-series-product: Tests shouldn't test for implementation details (slices, digits) largest-series-product: Should tests test for implementation details (slices, digits)? Mar 8, 2016
@kytrinyx
Copy link
Member

kytrinyx commented Mar 8, 2016

I strongly support the suggestion of not forcing students towards a given implementation. If we find that it's too hard, then we could add hints like the elixir zipper problem.

@kytrinyx
Copy link
Member

kytrinyx commented Mar 8, 2016

Pinging @exercism/track-maintainers in case you're not watching the repo :)

@petertseng
Copy link
Member Author

To my knowledge, all tracks no longer test for implementation details. Many thanks to all reviewers of the associated PRs.

@kytrinyx
Copy link
Member

Thanks so much for taking the time to get all of this cleaned up!

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

No branches or pull requests

2 participants