Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

high-scores: New exercise #1378

Merged
merged 1 commit into from
Oct 26, 2018
Merged

high-scores: New exercise #1378

merged 1 commit into from
Oct 26, 2018

Conversation

pgaspar
Copy link
Member

@pgaspar pgaspar commented Oct 24, 2018

An introductory Array-focused exercise me and @F3PiX worked on.

In the Ruby track, we're lacking an easy exercise with an array as a list and some list manipulations.
This exercise is meant to fill that gap, and may be interesting for other tracks to implement as well.
We paid extra attention to the sequence of the tests, so the code is building up nicely, and at the end there should be some nice mentoring opportunities left.
It's set up for incremental progress, where students can both learn new things and implement lessons learned; in the Ruby track, this will be the first exercise where they work with multiple methods, and the string interpolation that they know from TwoFer is here too, but now they have to prevent double spaces.

We created the Ruby implementation (exercism/ruby#886) at the same time, so we were probably influenced by it when creating this generic specification - please help us improve it 👍

We've also submitted a logo request here.

@ErikSchierboom
Copy link
Member

Love the idea for a simpler array exercise! This is something that the C# also lacks.

@@ -0,0 +1,3 @@
Manage a game player's High Score list.

The high score list knows about the highest scores, the last added score and other info useful to a game player.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to see more description of what the task actually is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @Insti, for the review, much appreciated!
How about this for the description:

The high score list knows about the highest scores, the last added score and other info useful to a game player.
Your task is to write methods that return the highest score from the list, the last added score, the three highest scores, and a report on the difference between the last and the highest scores. 

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a step in the right direction.

It's nice to provide a little more context and "color"

Have look at the description for exercises like book-store or kindergarten-garden or two-bucket

"input": {
"scores": [2, 4, 0, 3, 7]
},
"expected": "Your latest score was 7. That's your personal best!"
Copy link
Contributor

@Insti Insti Oct 24, 2018

Choose a reason for hiding this comment

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

Something about this bugs me. I think it is that this property returns a processed string while the others are arrays.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe if the property was something like message or goal_report that made it a bit clearer that we will be expecting a string rather than a numeric value.

Copy link
Contributor

Choose a reason for hiding this comment

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

encouraging_message?

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it even need to be a string?
Could we make it a boolean? personal_best

What is it you're trying to help teach?

Copy link
Contributor

@emcoding emcoding Oct 24, 2018

Choose a reason for hiding this comment

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

I do like report much better! We'll change that, thanks.

We actually want to keep the string, for several reasons. One is that it builds upon lessons learned, i.e. managing the string in TwoFer. In this exercise, it's a bit harder than in TwoFer to compose one string out of 2 cases (In Ruby there's 'preventing double spaces' with string interpolation) or handling duplication in the strings. Which doesn't justify an exercise in itself, so I wanted it integrated here.
Let me know if you want me to elaborate on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about making all the return values interpolated strings?

Copy link
Contributor

Choose a reason for hiding this comment

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

What about making all the return values interpolated strings?

This does kind of obfuscate the "simple array processing" aspect of the exercise.

"input": {
"scores": [3, 5, 2, 7]
},
"expected": [3, 5, 2, 7]
Copy link
Contributor

Choose a reason for hiding this comment

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

These scores seem low for 'high' scores.
Perhaps you could help this by just putting a line in the readme that says something like "Scores are usually in millions but we're leaving that bit off." or "It's really hard to get Frogs across the road so 7 is a good score!"

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm 🤔
We just went with scores that are easy to read, even in a longer list. And in general, we actually don't care much. Do you mind if we leave them?
Also, I'd rather not add it to the Readme.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had a good laugh with "It's really hard to get Frogs across the road so 7 is a good score!" 😄 However the exercise only refers Frogger in the source bit, which is relatively hidden.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather not add it to the Readme.

That's fine ❤️
(It is also fine if you don't want any of the "color" I was suggesting in another comment and and just want to retain a short to the point readme.)

@iHiD
Copy link
Member

iHiD commented Oct 24, 2018

Firstly, this is great :)

I've read through the comments and tend to agree with @Insti's vibe about the task side. I think I'd like to make it more about Frogger. It feels more fun that way. Maybe have some text in the description such as "Your task is to build the high-score component of the classic Frogger game, one of the highest selling and addictive games of all time, and a classic of the arcade era"

I would also then like to see the numbers with an extra 0 on the end. In Frogger, every step is worth 10points, so updating the tests so their all one OOM larger would be a simple but nice step. I actually would argue for lots higher numbers (in the region of 10k), but I'm marginally less fussed about that.

I've been generally thinking about "extension" tasks of exercises, where there is an additional "hidden" test that is somehow unlocked (mechanism and plausability tbd!) but for this exercise, we could have an extra thing where the high-scores then jump to the 10k range, and part of the task is to return strings with delimiters in the correct place (e.g. "28,310")

@pgaspar
Copy link
Member Author

pgaspar commented Oct 24, 2018

In Frogger, every step is worth 10points

I completely ignored that, but that's a great catch - thanks @iHiD. I'll add a zero for now.

As for the idea of making the Frogger theme more prominent and the exercise extension, I'll defer to @F3PiX who originally thought up the exercise 👍

They're interesting ideas.

pgaspar added a commit to pgaspar/ruby that referenced this pull request Oct 24, 2018
"description": "Returns the goal when highest is latest",
"property": "goal",
"description": "Returns report when highest is latest",
"property": "report",
"input": {
"scores": [2, 4, 0, 3, 7]
},
"expected": "Your latest score was 7. That's your personal best!"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want a different message in this case?
Something like "That is a new personal best!" maybe.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe we wanted to keep the number of possibilities on the report method relatively short (as in 2 😅). @F3PiX may have more context to that.

@Insti
Copy link
Contributor

Insti commented Oct 25, 2018

None of my suggestions should be considered blockers. It's would be fine to merge this and address tweaks through future pull requests.

@emcoding
Copy link
Contributor

emcoding commented Oct 25, 2018

@iHiD Great idea for the string formatting. I'm going to use that in a next exercise, that's slightly more advanced, and where it's a really really great feature, that just finishes it off 😍 I feel it's out of scope, at least stretching the scope, of the HighScore exercise.

@Insti @iHiD I added some extra Frogger colour to the description, without stretching it too much, because 'hi score' is actually the only thing this exercise and Frogger has in common.

@pgaspar pgaspar force-pushed the high-scores branch 2 times, most recently from 082703b to 7fe0c63 Compare October 25, 2018 19:28
"expected": [30, 50, 20, 70]
},
{
"description": "Lastest score",
Copy link
Member

Choose a reason for hiding this comment

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

Lastest -> Latest

@pgaspar
Copy link
Member Author

pgaspar commented Oct 25, 2018

Folks, I've squashed everything down to a single commit. Thank you for your reviews so far!

Co-authored-by: Maud de Vries <[email protected]>
Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Great work!

@emcoding
Copy link
Contributor

Thanks all for your inputs. Can we merge this?

Copy link
Member

@rpottsoh rpottsoh left a comment

Choose a reason for hiding this comment

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

@F3PiX and @pgaspar thanks for working on this. 👍

@Insti Insti merged commit 3204fc2 into exercism:master Oct 26, 2018
@Insti
Copy link
Contributor

Insti commented Oct 26, 2018

I can merge it. Thanks everyone ❤️

@emcoding emcoding deleted the high-scores branch October 26, 2018 10:51
@emcoding
Copy link
Contributor

THANKS All !!

@pgaspar
Copy link
Member Author

pgaspar commented Oct 26, 2018

Thank you!

matthewmorgan pushed a commit to exercism/javascript that referenced this pull request Oct 30, 2018
* Add High Scores exercise

New problem specification added in
exercism/problem-specifications#1378

* Set high-scores unlocked_by to pangram

* Use JavaScript getters in high-scores example solution

* Use Array spread instead of Array.slice

Co-Authored-By: PakkuDon <[email protected]>
@emcoding emcoding added the Track Anatomy Project Related to or helpful to the Track Anatomy Project label Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new exercise idea Track Anatomy Project Related to or helpful to the Track Anatomy Project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants