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

Concept maybe #361

Merged
merged 17 commits into from
Jun 23, 2021
Merged

Concept maybe #361

merged 17 commits into from
Jun 23, 2021

Conversation

mpizenberg
Copy link
Member

@mpizenberg mpizenberg commented Apr 30, 2021

No description provided.

@mpizenberg mpizenberg marked this pull request as ready for review May 28, 2021 12:49
Copy link
Contributor

@ceddlyburge ceddlyburge left a comment

Choose a reason for hiding this comment

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

Hi Matthieu, this looks good, and I would be happy to merge it.

One note is that I generally haven't been including function type signatures in the intial .elm file, and this one does have it. I'm not sure if we need to be consistent, but just flagging that we aren't.

Also, I take it that we are now planning an advanced Maybe concept? Where we can teach map and suchlike?

I wonder if it is worth teaching withDefault in this exercise. It is useful in the very basic 'two-fer' practice exercise, and is something that a lot of beginners don't know

Cheers, Cedd

@mpizenberg
Copy link
Member Author

Ha good points, I haven't had the time to check all your PR's yet, but I think except from the basics where we want students to write themselves the type signature it's good to provide a type signature for those template initial elm files. It usually makes the task clearer and also reduces potential types issues for the functions we import in the tests files. But of course we should only provides types of the exposed functions, not helper functions.

Regarding advanced concept exercises, I do thing we need few of them on most concepts like lists that I just reviewed and maybe. But you're probably right that withDefault could appear on that exercise without waiting on and advanced one. I think I just tried to not introduce functions from the module to focus on the core data structure. If you have an idea for introducing it here don't hesitate to update the exercise.

@mpizenberg
Copy link
Member Author

I wonder if it is worth teaching withDefault in this exercise

I've added a name : Maybe String field to the Player type and added a task to write

introduce : Player -> String
introduce { name } =
    Maybe.withDefault "Mighty Magician" name

Don't hesitate to merge if you like it or let me know otherwise.

@ceddlyburge ceddlyburge merged commit e3f1ff2 into main Jun 23, 2021
@ceddlyburge ceddlyburge deleted the concept-maybe branch June 23, 2021 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:size/large Large amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants