grade-school: fix student cannot be in multiple grades logic

[wolf99]

Fixes #1763, see the discussion there for explanation

[petertseng]

To me, I find this new case a0c7b9b8-0e89-47f8-8b4a-c50f885e79d1 equally as unsatisfying as the case that it reimplements c125dab7-2a53-492f-a99a-56ad511940d8 (Edit: the new case is unsatisfying, but less unsatisfying than the case it reimplements) because both of these cases can be passed by an implementation that declares this:

• a student that is added twice shall be considered to belong to neither of the grades they were attempted to be added to.

In other words, it does not seem to me that either the added case nor the case that it reimplements distinguishes between a desired behaviour and my above-described behaviour (and I presume my above-described behaviour is not what is intended/desired by the https://github.com/exercism/problem-specifications/blob/main/exercises/grade-school/description.md).

Thus, this newly-added case cannot receive my endorsement (and thus my GitHub Approval). But since there is a valid implementation (the one I described above) that passes it, I do not declare it incorrect, so it will also not receive my refusal. If others decide this case is the best solution to #1763 and the right way forward given the problems described therein, I do not oppose that determination - in such a case, please proceed without my Approval.

[petertseng]

Upon review, I specifically revoke the part of my previous comment where I claim the added case is equally as unsatisfying as the case it reimplements. I now notice that the reimplemented case is a roster with a desiredGrade, which is very much against the https://github.com/exercism/problem-specifications/blob/main/exercises/grade-school/description.md.

My standard for a GitHub Approval shall be whether a change is an improvement. The added case is unsatisfying (my reasons for declaring it such are unchanged), but it is less unsatisfying than the case it reimplements. Therefore, it receives an Approval.

[wolf99]

both of these cases can be passed by an implementation that declares this:

• a student that is added twice shall be considered to belong to neither of the grades they were attempted to be added to.

I agree that this is an issue. I will move this PR to draft and look at solutions.
Thanks @petertseng !

[wolf99]

I realise this still doesn't cover all all aspects of this condition. This is because the case is really an error condition for the action of adding students rather than of querying the roster property ... Fixing this will require the entire spec to be changed. I'm not averse to doing that but would like to get progress enough to fix #1763 and exercism/c#555 first though.

[petertseng]

We have an interesting situation: If we wish to signal explicitly that a case in canonical-data.json is no longer recommended, we can add a case that reimplements it. But I believe this is a situation where we might simply wish for a way to signal that c125dab7-2a53-492f-a99a-56ad511940d8 is no longer recommended and not provide a case that reimplements it. But we have no way to do that. I'll continue to Approve any valid and consistent cases that improve the situation. These two cases fit that criterion. Perfection isn't a requirement.

The reason I wished for a way to not provide a replacement has already been pointed out - we don't currently have a good way to be clear about the fact that this is an error that should happen at add time, not at query (roster/grade) time. We could:

1. Use a different property to signal that
2. Just put an error in the roster cases and expect tracks to do what makes the most sense for them
3. Choose not to specify any such cases in this canonical-data.json at all
4. Other solution I didn't think of

And, since c125dab7-2a53-492f-a99a-56ad511940d8 exists, doing 3 involves a way to signal its un-recommendation.

Note that we used to have a similar problem for bowling - if there are any invalid rolls, the errors should be signaled at roll time, not score time. This had to be fixed in #536.

[ErikSchierboom]

And, since c125dab7-2a53-492f-a99a-56ad511940d8 exists, doing 3 involves a way to signal its un-recommendation.

Note that we used to have a similar problem for bowling - if there are any invalid rolls, the errors should be signaled at roll time, not score time. This had to be fixed in #536.

Interesting problem. We indeed don't have a way to signify this "just don't use this test case anymore" scenario. Will perhaps be useful in the future. The only thing thats I could come up with are:

• Add comments (which don't show up in tests.toml files)
• Add a suffix to the title (which does show up in tests.toml files)

[SleeplessByte]

Thanks @wolf99. I like the new canonical data, and thanks everyone else for chiming in.