Skip to content

Implement High Scores exercise#1767

Closed
CodeMartian wants to merge 14 commits intoexercism:mainfrom
CodeMartian:master
Closed

Implement High Scores exercise#1767
CodeMartian wants to merge 14 commits intoexercism:mainfrom
CodeMartian:master

Conversation

@CodeMartian
Copy link
Copy Markdown

@CodeMartian CodeMartian commented Oct 4, 2019

Fixes #1746


Reviewer Resources:

Track Policies

@CodeMartian CodeMartian changed the title WIP - Implement High Scores exercise Implement High Scores exercise Oct 8, 2019
Copy link
Copy Markdown
Contributor

@lemoncurry lemoncurry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @JeffMartian , we get the following error messages for your PR that need to be resolved:
-> An implementation for 'gradle' was found, but config.json does not reference this exercise.
-> The implementation for 'gradle' is missing a README.
-> The implementation for 'gradle' is missing an example solution.
-> The implementation for 'gradle' is missing a test suite.

If you need any help or have any questions on how to implement an exercise for the Java track, please let us know. In CONTRIBUTING.md, we explain what steps are necessary to add a new exercise to the Java track.

@CodeMartian
Copy link
Copy Markdown
Author

Hi @lemoncurry! Yeah, I'm not sure why I'm getting those errors in the build.. I checked the config.json and ensured I have a high-scores entry in there. I also have a README, example solution, and a test suite for the exercise. Would you happen to have any advice or insight to these errors?

@bmuskalla
Copy link
Copy Markdown

bmuskalla commented Oct 16, 2019

@JeffMartian I'm not sure if this was on purpose but you seem to have added gradle binaries and a gradle wrapper script in the exercises/ folder

@jmrunkle
Copy link
Copy Markdown
Contributor

@lemoncurry - are you still available to do this review or should one of the other maintainers take over?

@jmrunkle
Copy link
Copy Markdown
Contributor

@lemoncurry - are you still available to do this review or should one of the other maintainers take over?

I'm taking that as a no. I'll takeover this one.

@jmrunkle jmrunkle requested review from jmrunkle and removed request for lemoncurry April 22, 2020 14:18
import static java.util.Arrays.copyOf;
import static java.util.Arrays.sort;

class HighScores {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: comments on this reference solution are just suggestions.

Comment on lines +11 to +15
```sh
gradle test
```

- Use gradlew.bat if you're on Windows
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regenerate this since we got rid of the advice to use gradlew.bat.

return personalBest.orElse(0);
}

Integer[] personalTopThree() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Integer[] personalTopThree() {
int[] personalTopThree() {

Everything else uses primitives - so why would we return a boxed array?

@@ -0,0 +1,21 @@
class HighScores {
public HighScores(int[] highScores) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think this should take a List since a player's high scores are likely to change over time. Arrays are better if the collection has a fixed size.

"slug": "high-scores",
"uuid": "574d6323-5ff5-4019-9ebe-0067daafba13",
"core": true,
"unlocked_by": null,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be unlocked by something. I think two-fer is a good one to unlock this.

"uuid": "574d6323-5ff5-4019-9ebe-0067daafba13",
"core": true,
"unlocked_by": null,
"difficulty": 1,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably difficulty 2 since it deals with storing member variables and then answering questions about those member variables.

NOTE: you should also move this down in the file into the difficulty 2 section (after the core exercise).

@Ignore("Remove to run test")
public void shouldReturnTopThreePersonalBestFromListOfScores() {
HighScores highScores = new HighScores(new int[] { 10, 30, 90, 30, 100, 20, 10, 0, 30, 40, 40, 70, 70 });
assertArrayEquals(new Integer[] { 100, 90, 70 }, highScores.personalTopThree());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You made these personalTopThree fit your reference solution (the one in .meta/), but not your implementation stub. The stub expects int[], but your reference solution returns Integer[]. To be honest, it really should be int[] since the rest of the class uses primitives.

@jmrunkle
Copy link
Copy Markdown
Contributor

@JeffMartian - are you still working on this PR?

@IA-JeffMartian
Copy link
Copy Markdown

@jmrunkle Yes I am! I haven't had a chance to make the requested changes yet though. I'm hoping to be able to get through it by the end of this week.

CodeMartian and others added 3 commits July 13, 2020 17:37
Co-authored-by: Jason Runkle <jmrunkle@gmail.com>
Co-authored-by: Jason Runkle <jmrunkle@gmail.com>
Co-authored-by: Jason Runkle <jmrunkle@gmail.com>
Base automatically changed from master to main January 28, 2021 19:15
@jmrunkle
Copy link
Copy Markdown
Contributor

I am so sorry that I lost track of this PR - any chance you are willing to synchronize this with the current state of the files? Otherwise I'll probably close the PR in the next week (you can always open a new one later).

@jmrunkle jmrunkle closed this Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

high-scores: implement exercise

5 participants