Skip to content

high-scores: Regenerate tests #936

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 13, 2019
Merged

high-scores: Regenerate tests #936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a62f49
high-scores: Regenerate tests
pgaspar Feb 10, 2019
bf79602
high-scores: Update example solution
pgaspar Feb 10, 2019
be6fcbf
high-scores: Update exercise to 4.0.0
pgaspar Feb 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions exercises/high-scores/.meta/solutions/high_scores.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,7 @@ def latest
scores.last
end

def personal_top
scores.sort.reverse.take(3)
end

def report
difference = "#{personal_best - latest} short of" if personal_best != latest

"Your latest score was #{latest}. That's #{difference} your personal best!".squeeze
def personal_top_three
scores.max(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'd prefer to have the 'minimal solution for approval', not the 'best'.
(Because it seems that some mentors refer to the example, or at least look at it.)

Copy link
Member Author

Choose a reason for hiding this comment

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

This shouldn't be too relevant 😕 These solutions are not for mentor consumption - and even if a mentor does see this I don't see it as a big issue at all. I'd rather keep things fairly separate - mentor notes are one thing, these solutions are another.

What do you think? Do you feel strongly about this? I can revert the change, but I don't want us to start changing these solutions every time we change the mentor notes - unless that's something we decide to do across all tracks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not very strongly, but let's say: preferrable. Strangely exactly because I agree with you: that we shouldn't start changing these solutions. Unless it's a bad example/ bad practice / outdated practice.
AFAIK the current line is none of those, or is it?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not. I'll change it back.

Copy link
Member Author

Choose a reason for hiding this comment

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

But really, the only reason I didn't commit max(3) the first time around is because I didn't know about it 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not. I'll change it back.

Actually, scores.sort.reverse.take(3) does do a bit of extra work, when compared to max(3).

Also, my point about not starting to change these solutions was specifically about changing them every time we change the mentor notes. I think we should be free to change these as we please, without the mentor notes having much influence. My main point is that (at least for now) they are different things and should change independently, if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooooh, this was absolutely not meant to be blocking merging this PR! I'm sorry.

end
end
2 changes: 1 addition & 1 deletion exercises/high-scores/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Manage a game player's High Score list.

Your task is to build a 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. 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.
Your task is to build a 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. Your task is to write methods that return the highest score from the list, the last added score and the three highest scores.

In this exercise you're going to instantiate a class and add some instance methods. http://ruby-for-beginners.rubymonstas.org/writing_classes/initializers.html

Expand Down
46 changes: 9 additions & 37 deletions exercises/high-scores/high_scores_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'minitest/autorun'
require_relative 'high_scores'

# Common test data version: 2.0.0 7a386a2
# Common test data version: 4.0.0 ad1f9c4
class HighScoresTest < Minitest::Test
def test_list_of_scores
# skip
Expand All @@ -24,66 +24,38 @@ def test_personal_best
assert_equal expected, HighScores.new(scores).personal_best
end

def test_personal_top
def test_personal_top_three_from_a_list_of_scores
skip
scores = [50, 30, 10]
expected = [50, 30, 10]
assert_equal expected, HighScores.new(scores).personal_top
scores = [10, 30, 90, 30, 100, 20, 10, 0, 30, 40, 40, 70, 70]
expected = [100, 90, 70]
assert_equal expected, HighScores.new(scores).personal_top_three
end

def test_personal_top_highest_to_lowest
skip
scores = [20, 10, 30]
expected = [30, 20, 10]
assert_equal expected, HighScores.new(scores).personal_top
assert_equal expected, HighScores.new(scores).personal_top_three
end

def test_personal_top_when_there_is_a_tie
skip
scores = [40, 20, 40, 30]
expected = [40, 40, 30]
assert_equal expected, HighScores.new(scores).personal_top
assert_equal expected, HighScores.new(scores).personal_top_three
end

def test_personal_top_when_there_are_less_than_3
skip
scores = [30, 70]
expected = [70, 30]
assert_equal expected, HighScores.new(scores).personal_top
assert_equal expected, HighScores.new(scores).personal_top_three
end

def test_personal_top_when_there_is_only_one
skip
scores = [40]
expected = [40]
assert_equal expected, HighScores.new(scores).personal_top
end

def test_personal_top_from_a_long_list
skip
scores = [10, 30, 90, 30, 100, 20, 10, 0, 30, 40, 40, 70, 70]
expected = [100, 90, 70]
assert_equal expected, HighScores.new(scores).personal_top
end

def test_message_for_new_personal_best
skip
scores = [20, 40, 0, 30, 70]
expected = "Your latest score was 70. That's your personal best!"
assert_equal expected, HighScores.new(scores).report
end

def test_message_when_latest_score_is_not_the_highest_score
skip
scores = [20, 100, 0, 30, 70]
expected = "Your latest score was 70. That's 30 short of your personal best!"
assert_equal expected, HighScores.new(scores).report
end

def test_message_for_repeated_personal_best
skip
scores = [20, 70, 50, 70, 30]
expected = "Your latest score was 30. That's 40 short of your personal best!"
assert_equal expected, HighScores.new(scores).report
assert_equal expected, HighScores.new(scores).personal_top_three
end
end