I suggest adjusting the last sentence of grade school as follows in order to align with tests [1], [2]:
new: “In case someone attempts to add the same student more than once, this student’s grade should be updated.”
old: “In fact, when a test attempts to add the same student more than once, your implementation should indicate that this is incorrect.”
This originated from a mentoring session in TS where a student rightly (imho) noted that the tests did not match the description.
Additionally (but optional imo), I suggest also adjusting the second to last sentence since the language is a bit ambiguous at the moment:
new: “Note that no two students have the same name (It’s a small town, what do you want?) and each student can only be added to exactly one grade on the roster.”
old: “Note that all our students only have one name (It’s a small town, what do you want?) and each student cannot be added more than once to a grade or the roster.”
Happy to put this into a PR.
P.S. I’m wondering whether a change to the central description (which is propagated to tracks if I understand correctly) should lead to (automatically create?) a PR in all tracks, copying the previous central version into an override - this way track maintainers can decide to either update tests where necessary and drop the PR or merge the PR (or of course adapt an existing override in some way).
This is where it becomes tricky, as the same instructions are shared across tracks. I agree that “should be ignored” might not be best way to phrase this, as some tracks might want to error or do something else. Could we come up with a track-agnostic description?
Just to be sure: how much should the tests in individual teacks correspond to the test specifications? These currently don’t error, but ignore the subsequent calls to add.
I guess in any case we’ll have to change the tests in TS since these seem to just be “wrong”…
Well, we don’t enforce anything. Tracks are free to deviate form instructions if that makes sense for their tracks. Most of the time, tracks stick quite closely to the instructions though.
You could look through the tracks and see how many implement the “ignore new adds” behavior. If a large majority do this, we may want to simply update the text to follow what is implemented already vs introducing “breaking” changes.
new "Any attempts to add the same student more than once should either be ignored or there should be an indication that this is incorrect - in a true TDD manner, check the tests to see what’s expected! .”
We also change the last sentence as suggested above
We change the TS tests to match the description
Agreed? If so, what do I do to make sure my PR s are not auto-closed?
Of the 68 tracks I have open, 40 implement grade-school. 24 of those implement 66c8e141-68ab-4a04-a15a-c28bc07fe6b9.
awk expects the second add to be a no-op (ignored)
c common-lisp crystal csharp elixir elm expects the second add to return a value indicating the student could not be added and have no side effect (no exceptions, no change to the roster)
I stopped there and did not check erlang fsharp gleam java nim perl5 powershell prolog python racket raku ruby swift tcl vbnet vlang wren
Based on the above, I think tests should probably ask that solutions not change anything on a second “add” and instead (where supported) return a value indicating the student is a duplicate and cannot be added.