-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Simplify High-scores exercise. Replace Class with functions #1764
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
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
f3fd126
Simplify High-scores exercise. Replace Class with functions
simmol e358937
Fix style problems
simmol 47d3eaf
Made requested changes in PR
simmol 7f9b8c6
Add new hints to high-score
simmol da39274
Merge branch 'master' into high_score_simplified
simmol c2bc97d
Remove changes to matrix from this PR
simmol 4317e1c
Update exercises/high-scores/.meta/HINTS.md
cmccandless 4d41a8e
Update exercises/matrix/README.md
cmccandless 91dabe3
Regenerate README
simmol e27148a
Merge branch 'master' into high_score_simplified
simmol c1e10a3
Merge branch 'master' into high_score_simplified
cmccandless File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,4 +8,5 @@ bin/configlet | |
bin/configlet.exe | ||
.idea/ | ||
.cache | ||
.pytest_cache | ||
__pycache__ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
In this exercise you're going to create a **class** and use lists to organize scores. _Don't worry, it's not as complicated as you think!_ | ||
In this exercise, you're going to use and manipulate lists. Python lists are very versatile, and you'll find yourself using them again and again in problems both simple and complex. | ||
|
||
- [**A First Look at Classes**](https://docs.python.org/3/tutorial/classes.html#a-first-look-at-classes) from the Python 3 documentation. | ||
- [**How to Define a Class in Python**](https://realpython.com/python3-object-oriented-programming/#how-to-define-a-class-in-python) from the Real Python website. | ||
- [**Data Structures in Python**](https://docs.python.org/3/tutorial/datastructures.html) from the Python 3 documentation. | ||
- [**Data Structures (Python 3 Documentation Tutorial)**](https://docs.python.org/3/tutorial/datastructures.html) | ||
- [**Lists and Tuples in Python (Real Python)**](https://realpython.com/python-lists-tuples/) | ||
- [**Python Lists (Google for Education)**](https://developers.google.com/edu/python/lists) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,10 @@ | ||
class HighScores(object): | ||
def __init__(self, scores): | ||
self.scores = scores | ||
def latest(scores): | ||
return scores[-1] | ||
|
||
def latest(self): | ||
return self.scores[-1] | ||
|
||
def personal_best(self): | ||
return max(self.scores) | ||
def personal_best(scores): | ||
return max(scores) | ||
|
||
def personal_top_three(self): | ||
return sorted(self.scores, reverse=True)[:3] | ||
|
||
def personal_top_three(scores): | ||
return sorted(scores, reverse=True)[:3] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,10 @@ | ||
class HighScores(object): | ||
def __init__(self, scores): | ||
pass | ||
def latest(scores): | ||
pass | ||
|
||
|
||
def personal_best(scores): | ||
pass | ||
|
||
|
||
def personal_top_three(scores): | ||
pass |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to ping @BethanyG for suggestions on new HINTS content. She put some good work into the current HINTS.md (less than a month ago), and I would feel bad about dumping that work without offering the chance to contribute an alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to write up some new hints...but I think the bigger issue here is that if you remove making a class in this exercise (which is a fairly easy one), then when students have to make a class (and deal with side effect questions) in the next exercise..well, we've just kicked the can down the road. The same issues that apply to High Scores apply to Matrix (and Kindergarten Garden, etc.) ...and eventually, a python learner has to to deal with classes. They're part of the language (it's really is objects all the way down), and widely used in frameworks like Django. The sooner students get (even a little) practice in a controlled fashion, the less they struggle later.
So the way I see it is that we either have to craft an exercise or two around classes and mutability specifically, or re-work tests and mentor notes on current exercises to address it. I don't think the answer is to re-write this exercise (and the next) to not need a class. If we don't want a class-based exercise here because we want to focus on lists, we could move this exercises to a later stage (and address its shortcomings through a re-write of tests). We could replace it with something like Say or Run Length Encoding - or any other number of exercises that focus on lists and arrays. But only 6 of 18 core exercises require writing a class currently, and I'm not sure we are doing a great job of giving students practice in that area.
All that being said....if we do indeed want to make this exercise function based, I can try to make some new hints this week. However, we then should move these hints to the Matrix exercise, since it will then be the exercise to introduce classes. We maybe should also provide a hint on mutability there as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies - forgot one last point: We'll need to re-work the mentor notes for this exercise as well. I don't think I will be able to get to either that or the hints until this afternoon, PST.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not opposed to that.
There are very few easy exercises as it is. I'd prefer not to overcomplicate what was meant to be a simple list exercise.
I can get behind that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok - I wasn't able to make suggestions in the pull (I don't have access) - but here's my proposed hint text:
In this exercise, you're going to use and manipulate lists. Python lists are very versatile, and you'll find yourself using them again and agin in problems both simple and complex.
I'll submit a pull for revised mentor notes and for the new hint on Matrix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See exercism/website-copy#956 for the updated mentor notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @BethanyG :)
I will add the new hints right now :)
And actually will push the old hints to Matrix in this PR. So we did not loose them.
For me Matrix is better Class based exercise. But we can make other also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind creating a separate PR for the changes to
matrix
? We try to limit PR changes to one exercise at a time (there are few exceptions).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well doesn't that count as exception? We have not changed the exercise in Matrix, just add the hints because previously they ware available at this exercise?
I can open other PR is not a big deal, but we have to merge them one after the other. I thought it would be better if we push this as one change.
I will make new one tomorrow morning and will edit this one to remove matrix docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have an issue with both exercises temporarily having the same generic hints about classes, so I went ahead and merged #1766.