Football Match Reports: Teach default keyword instead of exceptions

[AlvesJorge]

The Football match reports is a concept exercise teaching the switch statement. This precedes the 'exceptions' part of the track, so it feels odd that the exercise asks you to do something that you haven't learned before, and which is not an intuitive part of the language for beginner programmers.

In contrast, the exercise does not propose the use of the default keyword.

I changed it so that the exercise now wants you to return "unknown" for any shirt outside of the 1-11 range, instead of wanting you to throw an error.

@sanderploegsma had previously showed a similar sentiment in this issue and related forum thread, but got no replies.

On the same note, the break keyword is also not being "taught" here, most people are simply returning from the switch, or returning the switch itself. So there's an opportunity for change there as well, but I got no suggestions atm

[github-actions]

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

[sanderploegsma]

Leaving this for the active maintainers to review, but since I got pinged I'll leave my 2cts:

The fact that you as a learning student got stuck because you had to figure out exceptions by yourself instead of through a concept exercise is a good indication that this should be addressed.

As for the solution, I really like the approach. The only remark I have is that the current IllegalArgumentException provides better semantics than "unknown". Especially in the context of negative shirt numbers, returning "unknown" is a bit silly. My suggestion would be to have it return "invalid" for shirt numbers outside the range 1-11.

---

Edit: thanks for contributing, especially since you're using Exercism to learn Java! 😃

[kahgoh]

Thanks @sanderploegsma for the comment - I found it helpful in the review 😄. Also thanks @AlvesJorge for pointing this out and and improving things on the Java track!

I also like removing exceptions from here since there is a separate concept for it and removing it from here reduces the number of dependencies for the switch concept.

The changes generally look fine, but:

• The example solution will require updating to pass the test changes.
• Feel free to also to add yourself to the contributors in the exercise's config.json (see Knapsack's config.json for an example with contributors).

[kahgoh]

Looks good! Thanks @AlvesJorge!

[AlvesJorge]

@kahgoh I noticed the new exercise is published in the java track, but the existing solutions are still marked as "passed latest version of the exercise" (see screenshot).
Did we forget to do something to update that? :)

[jagdish-15]

@AlvesJorge

We deliberately included [no important files changed] in the merge commit message to prevent rerunning tests on already submitted solutions. Since the tests were updated, rerunning them would cause previously accepted solutions to fail, which wouldn’t be fair, as they were correct at the time of submission.

This way, the changes only apply to solutions submitted from now onwards. So nothing was forgotten, it’s actually by design! 🙂

[kahgoh]

👆🏻 that and to properly mark them as no longer passing requires re-testing them all, which can be a rather expensive operation (more info in the docs if you're interested).

[AlvesJorge]

Yeah I understand the not wanting to check if all submissions pass the latest version. It's just that they explicitly say "This solution correctly solves the latest version of the exercise". Which is awfully specific, and very wrong. If i was a newbie and I copy pasted the solution code I'd be so confused as to why it's not passing.

[jagdish-15]

Are you sure it shows that? As far as I know, and even after checking, it doesn’t seem to. Could you double-check on your side?
@kahgoh do you happen to know more about this?

Edit — Ah, you’re right! It does say that, just realised it now after @kahgoh mentioned it shows up when hovering on the check mark.

[kahgoh]

Yeah ... I see it ... the tooltip does say that when hovering over the check mark. I can see how it can be confusing and using [no important files changed] would have prevented it from re-running the check.

On another note ... I wonder if the test display name "The onField method returns 'invalid' for invalid shirt number" should describe why it is invalid (e.g. "The onField method returns 'invalid' for shirt positive number outside range"). It would be more consistent with the one below it which says "negative number". If so, we could make the change and re-run the check at that point (@AlvesJorge, if agreed, would you want to raise the PR?), otherwise I'll see if someone can trigger the re-testing.