Could the grade-school tests be less prescriptive?

[thomanski]

I'm working through the Rust exercisms and have so far enjoyed the journey. grade-school led to some frustration though. At first I thought that was just down to me trying to program on the train home after having had a few beers, but when I decided (which I hadn't done on any other of the exercises so far) to just submit an empty solution in order to see what others have done I couldn't help but notice I wasn't alone in my struggles, others commented on/criticised the tests as follows:

[...], but that requires rewriting the tests to take ownership.
http://exercism.io/submissions/0e24aaf3d672454eb536acdba0dd7073

// test forces me to use Vec instead of BTreeSet
--> http://exercism.io/submissions/604fd3bd27614f98a939eeb40428f70d

The test cases here made it such that one was expected to solve this problem in one way. [...]
--> http://exercism.io/submissions/3f9fc61277c94385be7338fb2c8e04e4

I gave up...
--> http://exercism.io/submissions/c11bd73b8f3c48dcb23c4dba66cc0d41

I hated this example. The tests have been written in such a way that they assume very specific function definitions, [...]
--> http://exercism.io/submissions/948e0931d14d42bbb11dcc4df01f09c8

// I would prefer to use a BTreeSet here, or at least
// BinaryHeap, but the test definition seems to make it
// impossible [...]
--> http://exercism.io/submissions/845cdbef93374155ad7632c9f339aabf

[IanWhitney]

What API do you think we should use in the tests? Grade-school predates my time working on this track, so I don't know the history. But, generally, we try to avoid writing tests that require specific implementations.

[petertseng]

(I see I was beaten to commenting, but I'll add what I have to say)

Thanks for the report from the ground. The policy of Exercism is that the tests should not be over-prescriptive: exercism/discussions#44. Therefore, we should move in the direction indicated by this issue.

The next step is to figure out how we can make it better. I can try to complete this exercise myself (as you can see, I have not) to see if I notice any specific recommendations to make. But anyone else can feel free to beat me to the punch in making suggestions here.

[thomanski]

Hey guys, thanks for the swift responses and all the great work you do on here! It's great to see how the Rust track has developed over the last year. I started over a year ago then had a long break before returning recently.

I wish I had useful/concrete suggestions but sadly I don't. I do plan to revisit the exercise at a later point though.

It just occurred to me that we could make the other commenters aware: @tokenrove @dewidaniels @pviecelli @chrisvittal @aslpavel @glennpratt @DuBistKomisch

[petertseng]

Mostly unfiltered comments from the time of me fetching the exercise until I got the first test passing:

No stub provided, makes me a little sad. Time to figure out what sigs the test wants.

• School::new, okay, standard stuff...
• okay, add(&mut self, n: usize, s: &str), no surprises here.
• okay, grade(&self, n: usize) -> Option<Vec<_>> - erm, what goes in that vec again? okay, we map stringvec_to_strvec over the option, and that takes &Vec<String>, so it should be Option<&Vec<String>>.
• and finally, grades(&self) -> Vec<usize>, fine.

Observations at this point:

• grades seems to depart from the README a bit. The README implies that grades would return a map of grade -> Vec<student> or something. Here, we are only obligated to return Vec<usize>. Not necessarily advocating for that to change - note that if it changes to match the README it will probably make the exercise harder.
• I understand why one would want Option because you want to clearly indicate the case when the grade doesn't exist, rather than returning an empty vector. Unfortunately, this puts a hole in one solution idea that I had - something like let v: Vec<_> = school.iter_students_of_grade(3).collect() - that way as long as iter_students_of_grade returns an iterator, students can use a vec or a tree or whatever (but remember, returning an iterator is hard)
• Erm, okay, if returning an iterator is so hard, another alternative I have in mind is school.for_each_student_in_grade(3, |s| ... ) Caller passes a closure, which will be called for each student in the grade. It's a bit of an unusual interface...
• Another idea: just call iter().collect() on whatever is returned by grades() and grade(n). Then no assumptions are made about exactly what kind of iterable thing is returned by those two functions.

[thomanski]

That sounds pretty clever.

[petertseng]

One question I would ask is - there are a few other problems that ask for Vecs to be returned: anagram, dominoes, pascals-triangle, sieve, variable-length-quantity. What makes grade-school different?

I thought of two answers, myself. Could someone familiar with the subject matter add more to help me?

• Signatures are not provided, and mapping stringvec_to_strvec over an option obscures the expected return type of grade.
  • This can be mitigated by, of course, providing the signatures. We discussed this in the past, in Provide function signatures  #117 and Are stub lib.rs files a good idea? #200.
  • Note, however, that if we follow the recommendation of being more agonostic about what the return type is (by calling iter().collect() on it), then we will ironically be over-constraining students again by providing the exact signature (since we have to provide some signature, and we cannot return an iterator)
  • The only solution I have to that is to also provide a comment saying "You can change this to be something other than Vec if you want", but I don't know how many want to do that.
• Vec is not the internal representation that students want to use.
  • Devil's advocate: An interface is supposed to abstract away implementation details! Just because the grade(n) function has to return a vector doesn't mean that the internal representation has to be a vector.
    • Proven false after attempting to implement a solution based on BTreeSets. grade(n) has to return an Option<&Vec<Student>>. Notice the & on that vector. The school has to own it so that it can lend it out. If it were an Option<Vec<Student>>, then the school can be free to create whatever vector it wants on the fly, but requiring an Option<&Vec<_>> means the school has to maintain some vectors somewhere.

---

Given what I've seen, my first recommendations would be:

• Provide a stub file.
• Change grade to return Option<Vec<String>> - no &. Then students are free to use whatever internal representation as desired.
  • My earlier alternative calling iter().collect() on the returned value is no longer my top recommendation: The benefit from that is too small because we are providing a stub file.
  • My earlier alternative of having grade return an iterator is still technically possible, but may be a bit much to ask. I would recommend moving the exercise later in such a case.

[DuBistKomisch]

I agree with grade -> Option<Vec<String>>. I don't think requiring Vecs is any worse (i.e. more prescriptive) than any other problem. You can always transform your internal representation into a Vec in the method.

Returning iterators should probably only be considered once Rust adds a tidier way to do it. I ended up doing it internally in a later problem and it was a PITA.

[tokenrove]

Change grade to return Option<Vec> - no &

This seems to me to be the key change, but I didn't feel comfortable enough with Rust to suggest it here.

If the point is of the exercise is to teach about lifetimes and avoiding copies, an iterator would make more sense than returning a reference to an internal data structure, although it sounds like this is too painful for an early exercise.

It would be nice if, when a constraining choice like this is made, the README or stub file could be explicit about why that interface was chosen.

[thomanski]

Looks like consensus then is to move to returning Option<Vec<String>>.

Now that I've done the exercise again I'm not sure we need stubs. Seems like a good exercise to me, having to figure that out. Then again I was struggling with that before we had this conversation...

[petertseng]

All interested are invited to review #256.