-
-
Notifications
You must be signed in to change notification settings - Fork 405
Add new concept mapsets
and concept exercise gotta-snatch-em-all
#1578
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
Conversation
Thank you for contributing to Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:
Automated comment created by PR Commenter 🤖. |
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.
Excellent exercise, very good about.me
, I had no trouble solving it and learned new things 🙌
- Use [this `MapSet` function][put] | ||
- Use [that `MapSet` function][member] |
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.
Nitpick: It would be nice if there was a better indicator of the difference between those two functions than just "this" and "that". Maybe describing briefly their purpose, like the other links? "To add card to a collection...", "To check whether...".
(Same nitpick about step 5 hints)
You cannot trade a card you don't have, and you shouldn't trade a card for one that you already have. | ||
|
||
Implement `trade_card`, that takes two cards to trade (yours and theirs) and your current collection. | ||
The return value is a tuple of two values: a `Bool` stating if the trade is possible and worth doing, and the collection you would end up with if you did the trade (even if it's not actually possible). |
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.
Nitpick: Bool
is a description for the boolean type that I would expect in an object-oriented programming language that literally has a class called Bool
.
The return value is a tuple of two values: a `Bool` stating if the trade is possible and worth doing, and the collection you would end up with if you did the trade (even if it's not actually possible). | |
The return value is a tuple of two values: a boolean stating if the trade is possible and worth doing, and the collection you would end up with if you did the trade (even if it's not actually possible). |
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.
Nice catch, it's a leftover from the Elm instructions, where Bool
is a type.
Implement `new_collection`, which transforms a card into a collection. | ||
|
||
```elixir | ||
new_collection("Newthree") |
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 think in all Elixir concept exercises, we demo the function calls using the module name too
new_collection("Newthree") | |
GottaSnatchEmAll.new_collection("Newthree") |
|
||
```elixir | ||
MapSet.new() | ||
# => MapSet.new([]) |
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 had to do a double-take to believe that this is how the REPL prints that value 😂
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 think it used to be something like #MapSet<>
or whatever, but they've been moving away from these notations to things that you can copy/paste. In that sense, this is the best way to represent sets :)
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.
That's so clever! I've always been annoyed at those #
things that I couldn't copy-paste. I think they showed up most often for Ecto structs.
config.json
Outdated
"pattern-matching", | ||
"booleans" | ||
], | ||
"status": "beta" |
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.
Lol, there are concept exercises are in "beta"? Maybe we should just make all of the "active"
Co-authored-by: Angelika Cathor <angelikatyborska@fastmail.com>
Sorry, but this was not about you, it was about sets! 🤣 I'm glad you like the PR, thanks for the review, I've addressed your comments in the relevantly named commits. |
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.
🚢
This PR add new concept and concept exercise, both forked from the Elm track.
Full disclosure, I used AI (via Cursor) to help translate from one language to the other, and event though I combed through everything and edited a lot of things, it was a huge time saver.
There might be stylistic differences between tracks that I'm blind to, I'm happy to change anything.
The CI is expected to fail until the
about.md
is finalized, then I'll copy it tointroduction.md
(let me know if you think some parts should be cut down) and use the generator to add the intro file to the exercise.