Sync high-scores

[IsaacUtah1379]

The instructions for HighScores say to write methods returning the desired values, but the tests expect the values to be stored as properties. Additionally, the instructions are somewhat ambiguous.

[mk-mxp]

Hi @IsaacUtah1379 and welcome to the PHP track!

Thanks for raising this issue. This exercise description is synchronised to the Exercism wide problem specifications. The problem-specification repository defines descriptions and test values for 77 programming languages. Sometimes that means, a language does not 100% fit to what is described.

We plan for this exercise to suggest PHP 8.4 property hooks, so you somehow actually do write methods instead of just setting properties. Does that, in your eyes, help to overcome the gap between description and implementation?

[IsaacUtah1379]

Yes, that makes much more sense now. Is there any way I can help with that?

[mk-mxp]

You may try to setup the PHP track contribution dependencies as described in the README and completely synchronize the exercise as outlined in #913 The issue contains adding the property hooks to the instructions.

If that is too much, or you cannot setup the dependencies, then you still can create the instructions.append.md as outlined in the issue and submit it for merging. Every bit of help is welcomed!

[IsaacUtah1379]

I've run configlet and added instructions.append.md, but I don't entirely understand how to do the rest. If you're willing to walk me through it, I'd love to learn, but I understand if you'd rather merge now and have someone else who knows what they're doing take care of the rest.

[mk-mxp]

I'll walk you through the whole thing 😄

• Comment in Sync high-scores #913, so I can assign you the issue
• Take a look at Sync hello-world #909, where most of the synchronisation steps were done, too
• The source of the test cases is canonical-data.json in problem-specification repository
• configlet sync ... created a .meta/tests.toml file, which provides the UUID to use in the DocBlock comment and the description to use in #TestDox(). Also use that description for the test method names. There is a "Top 3 Scores" part not present in the current method names you should add
• The test values in canonical-data and our tests should match and the order of tests should also match
• You can run composer test:run -- highscores and composer ci to test your changes locally

I haven't seen any other things I think need to be explained. But of course, if there is still something unclear, please ask

[IsaacUtah1379]

Sweet! Thanks for all your help! I will admit I don't know what the students stub mentioned in #913 is, and I don't know what to do with the last step that says "Decide on adding / adjusting / ordering test cases to match current problem specs". Would you mind clarifying what is meant by these?

[mk-mxp]

@IsaacUtah1379 The "students stub" is the file, the students get to start programming from. It's exercises/practice/high-scores/HighScores.php here.

The "Decide on adding / adjusting / ordering test cases to match current problem specs":

During the previous steps you synchronise all the tests we already have. But sometimes there are more test cases in the problem specs. If we add them, do we break existing solutions? Then we need to decide, if we want to add them.

Same for changing the values in tests - if existing solutions might break using the new values, we should discuss adjustment first. This is rare, but can happen.

Ordering the tests we have according to the order in problem spec usually does not break anything, but sometimes there are other reasons. E.g. we may have a certain test fail before another so students can see the real cause of the failure better. So we should discuss before re-ordering is done. Also re-ordering usually leads to hard to follow change sets in the PR.

[IsaacUtah1379]

Syncing the tests with canonical-data.json would require reordering, modifying, and adding tests. Who should I discuss this with?

[mk-mxp]

We will discuss that here. Adjusting the values is usually no problem, when that does not change limitations or add new constraints. You may change the values you think are OK. Mention the others here, or wait for me to do a proper review on the weekend.

What new tests are causing the current solutions to fail? And why? I currently don't have the time to take a deeper look myself, you may add them and comment them out so I can take a quick look. Or wait until I find time for the review.

[IsaacUtah1379]

Actually, none of the tests are causing failures. I didn't think to run the composer tests before worrying about the tests. If there's anything specific you need to look at, let me know, but the tests all pass for this exercise.