New concept exercise: `new-passport` (concept: `with`) #954

jiegillet · 2021-10-10T14:59:12Z

Daft for #560.
I had a fun idea so I put a draft together. I only added the empty files, design.md and the exemplar for now.

The idea is about dealing with the frustrations of getting a new passport from the city office. First, you have to get the time right (they are basically never open), you have to find the right counter to go to (the information desk takes long coffee breaks), the counter you should go to depends on your age, and you better have brought the form of the right color!

With so many potential point of failures, you better use with.
Let me know what you think of the idea and I'll work on it more.

jiegillet · 2021-10-11T14:30:13Z

Added tests.
The CI fails, maybe because DateTime.add was added in 1.8? I'll fix it later

angelikatyborska

Exquisite story!

exercises/concept/new-passport/lib/new_passport.ex

angelikatyborska · 2021-10-11T19:19:54Z

exercises/concept/new-passport/lib/new_passport.ex

+    if time_between(time, ~T[14:00:00], ~T[14:20:00]) do
+      {:coffee_break, "information counter staff on coffee break, come back in 15 minutes"}


What a realistic error case, I love it 😂

Saw this fly by, and had to add how perfect the story is (and the error case!). I think the Python track is totally forking this. But being from the US, I would probably make it about the DMV (department of motor vehicles). 😂

Hahaha I'm glad you all like my story :)

exercises/concept/new-passport/lib/new_passport.ex

jiegillet · 2021-10-12T13:53:46Z

I had to stop using DateTime because ~U was introduced in 1.9 :(

concepts/with/introduction.md

concepts/with/links.json

exercises/concept/new-passport/.meta/exemplar.ex

neenjaw · 2021-10-14T04:56:43Z

Added tests. The CI fails, maybe because DateTime.add was added in 1.8? I'll fix it later

@angelikatyborska, How long did we say we would guarantee compatibility for? 1.9 was released Jun 24, 2019 (> 2 yrs). I can't find the exact cut off, but I think one criteria was when we hit features that prevented us from writing current idiomatic elixir.

Should we drop 1.7/1.8?

angelikatyborska · 2021-10-14T06:58:56Z

We wrote in CONTRIBUTING.md that we want to support the latest 6 minor versions, which would still include 1.7 and 1.8. Jie already switched from DateTime to NaiveDateTime, so it's no longer a problem in this PR. I would argue that NaiveDateTime is better in this case anyway because we're being explicit about not handling timezones.

jiegillet · 2021-10-15T13:24:03Z

Ok, I think it's ready for review.

Some things I'm unsure about:

My hints or maybe the difficulty balance between tasks? I realized that only task 2 has any meat. Everywhere else the hints are stating the obvious. Not sure if it's a problem.
Which exercise to add as practicing with?
- To me phone-number is an obvious example of using with, but I didn't find any other people using it.
- I used it a lot in sgf-parsing but @neenjaw (only other solution so far) never touched it
- I grepped all exemploid (™️) solutions that use with, only sgf-parsing and variable-length-quantity but the latter hardly qualifies as a prerequisite even less a practice.

I still like with even if it's not super popular though, probably because it is a mini-monad 🐱

angelikatyborska

Big review incoming! I left a lot of suggestions for the instructions, I think they could be improved, and the topic of <- vs = needs to be clarified. Otherwise looks good. Great job, it will be an awesome exercise! I love that it also kinda "practices" NaiveDateTime 🙂.

My hints or maybe the difficulty balance between tasks? I realized that only task 2 has any meat. Everywhere else the hints are stating the obvious. Not sure if it's a problem.

I didn't have that feeling when solving, but I did go against the instructions and started writing a with right away. Maybe the instructions could suggest doing that, starting with with right away? Not because it's needed to fulfill task 1, but as a preparation for other tasks.

I found myself not changing the return value of the function until the last task because none of the tests cared. I also noticed that the step counter = birthday_to_counter.(birthday), even though listed as under task 2, doesn't need to be added to make task 2 tests pass.

One thing that is a bit unusual about this exercise is that the instructions have no return value examples. That probably cannot be added if only the last step makes the function return the correct "happy path" value? Or maybe the error paths can be shown instead?

To me phone-number is an obvious example of using with, but I didn't find any other people using it.

I think that could qualify it to deserve having "with" as a prerequisite, same thing about sgf-parsing (btw wow, only the two of you solved that exercise).

I don't think we can find something that practices "with" because it only really shines when dealing with a lot of uncertainty, like external API calls, something that Exercism doesn't practice.

exercises/concept/new-passport/.docs/instructions.md

exercises/concept/new-passport/.docs/introduction.md

angelikatyborska · 2021-10-16T10:55:19Z

exercises/concept/new-passport/.docs/instructions.md

+
+## 2. Go to the information desk and find which counter you should go to
+
+The staff at the information desk is notoriously lazy, they won't do any work during their sacred, excruciatingly long, coffee breaks. If you are lucky enough to find someone on duty, you still need to understand the instructions for which counter to go to. It is so complex that they need to give visitors an instruction manual with the algorithm.


General feedback about all the tasks descriptions: could we tone down the contempt for the city employees? 😁 I know it's funny to complain about the convoluted process and bureaucracy, but I don't want to offend anyone by calling them lazy. I know from some of my friends and family how terribly underpaid and mistreated some of those workers are.

When I solve those exercises, I find that it flows better then the task instructions:

have all of the necessary information to do the task (so I don't need to reference anything from the top of the document)

keep the amount of unimportant information to the minimum (so the story text is only limited to briefly explaining why the function is needed without adding too much irrelevant background information)

You are right, I got caught up in my own comedy show. I toned down the complaints and the jokes, hopefully it's better now.

exercises/concept/new-passport/.docs/instructions.md

exercises/concept/new-passport/.meta/exemplar.ex

exercises/concept/new-passport/.docs/instructions.md

exercises/concept/new-passport/.meta/design.md

config.json

concepts/with/links.json

jiegillet · 2021-10-16T14:51:07Z

That is THE single largest most detailed review I've ever got. Thank you for taking the time and leaving all those actionable suggestions :)

github-actions · 2021-10-17T06:37:07Z

Thank you for contributing to exercism/elixir 💜 🎉. This is an automated PR comment 🤖 for the maintainers of this repository that helps with the PR review process. You can safely ignore it and wait for a maintainer to review your changes.

Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:

General steps
- 🏆 Does this PR need to receive a label with a reputation modifier (x:size/{tiny,small,medium,large,massive})? (A medium reputation amount is awarded by default, see docs)
Any exercise changed
- 👤 Does the author of the PR need to be added as an author or contributor in <exercise>/.meta/config.json (see docs)?
- 🔬 Do the analyzer and the analyzer comments exist for this exercise? Do they need to be changed?
- 📜 Does the design file (<exercise>/.meta/design.md) need to be updated to document new implementation decisions?
Concept exercise changed
- 🌲 Do prerequisites and practices in config.json need to be updated?
- 📖 Does the concept introduction provide all necessary information to solve this exercise?
Concept exercise tests changed
- ⚪️ Are all tests un-skipped?
- 🔢 Are all tests annotated with @tag task_id?
- 🐈 Can all tests be understood by reading the test's block only (e.g. no module attributes, no setup functions)?
Concept changed
- 👤 Does the author of the PR need to be added as an author or contributor in <concept>/.meta/config.json (see docs)?
- 📖 Does the concept introduction provide all necessary information to solve this exercise?

Automated comment created by PR Commenter 🤖.

jiegillet · 2021-10-17T07:21:11Z

I found myself not changing the return value of the function until the last task because none of the tests cared. I also noticed that the step counter = birthday_to_counter.(birthday), even though listed as under task 2, doesn't need to be added to make task 2 tests pass.

One thing that is a bit unusual about this exercise is that the instructions have no return value examples. That probably cannot be added if only the last step makes the function return the correct "happy path" value? Or maybe the error paths can be shown instead?

That is quite unusual indeed, but I couldn't really find ways to get around it that would make the firsts tests still pass after modifying the function. I suppose we could build 1 function per task but that would feel clunky.

Maybe I could add a step go_to_counter(now, birthday, counter) that checks that it is correct? It wouldn't add so much to the exercise, but it also doesn't hurt.

angelikatyborska

I think I did a final review 🎉. I still left a few small suggestions but I'm approving already. Feel free to merge when they're resolved. I'm sure people will find other problems instantly after publishing the exercise 🙈 . In the beta phase of the platform, we had at least some time to test them ourselves before publishing.

That is quite unusual indeed, but I couldn't really find ways to get around it that would make the firsts tests still pass after modifying the function. I suppose we could build 1 function per task but that would feel clunky.

Yeah, I guess it has to stay like that.

Maybe I could add a step go_to_counter(now, birthday, counter) that checks that it is correct? It wouldn't add so much to the exercise, but it also doesn't hurt.

I might have lost the thread in the conversation here. What is this referring to, what are you suggesting?

angelikatyborska · 2021-10-20T06:38:52Z

concepts/with/introduction.md

+     {:ok, avatar} when is_bitstring(avatar) <- fetch_avatar(id),
+     avatar_size = bit_size(avatar),
+     {:ok, image_type} <- check_valid_image_type(avatar) do
+  {:ok, image_type, avatar_size, avatar}
+end
+```
+
+At each step, if a clause matches, the chain will continue until the `do` block is executed. If one match fails, the chain stops and the non-matching clause is returned. You have the option of using an `else` block to catch failed matches and modify the return value.
+
+```elixir
+with {:ok, id} <- get_id(username),
+     {:ok, avatar} when is_bitstring(avatar) <- fetch_avatar(id),
+     avatar_size = bit_size(avatar),


Since neither = or guards are explained here or even required to solve the exercise, we should drop them from the introduction. I'm thinking the first code example might not even be necessary, the example with else should be enough since the text already says that else is optional.

exercises/concept/new-passport/.docs/hints.md

angelikatyborska · 2021-10-20T06:41:45Z

exercises/concept/new-passport/.docs/instructions.md

+
+## 2. Go to the information desk and find which counter you should go to
+
+The information desk is notorious for taking long coffee breaks. If you are lucky enough to find someone there, they will give you an instruction manual which will explain which counter you need to go to depending on your birth date.


That's better 👍

exercises/concept/new-passport/lib/new_passport.ex

jiegillet · 2021-10-20T07:15:04Z

Maybe I could add a step go_to_counter(now, birthday, counter) that checks that it is correct? It wouldn't add so much to the exercise, but it also doesn't hurt.

I might have lost the thread in the conversation here. What is this referring to, what are you suggesting?

Haha, there were a lot of comments. I was answering to this:

I also noticed that the step counter = birthday_to_counter.(birthday), even though listed as under task 2, doesn't need to be added to make task 2 tests pass.

So I was considering adding an extra step to make sure that people got a meaningful value for counter. But again, I can't directly check the the values before the last step anyway, so it wouldn't be that helpful.

Co-authored-by: Angelika Tyborska <angelikatyborska@fastmail.com>

jiegillet · 2021-10-20T07:48:12Z

Alright, I'm merging. Thank you for the in-depth review.
Problems and suggestions for improvement will come indeed, and they will be welcome (by me, anyway) 👍

jiegillet added x:action/create Work on something from scratch x:size/large Large amount of work x:module/concept-exercise Work on Concept Exercises x:knowledge/intermediate Quite a bit of Exercism knowledge required labels Oct 10, 2021

jiegillet changed the title ~~New concept exercise with~~ New concept exercise: new-passport (concept: with) Oct 11, 2021

angelikatyborska reviewed Oct 11, 2021

View reviewed changes

jiegillet added the hacktoberfest-accepted label Oct 12, 2021

angelikatyborska reviewed Oct 13, 2021

View reviewed changes

concepts/with/introduction.md Outdated Show resolved Hide resolved

angelikatyborska reviewed Oct 13, 2021

View reviewed changes

concepts/with/links.json Outdated Show resolved Hide resolved

angelikatyborska reviewed Oct 13, 2021

View reviewed changes

exercises/concept/new-passport/.meta/exemplar.ex Outdated Show resolved Hide resolved

jiegillet marked this pull request as ready for review October 15, 2021 13:24

jiegillet added 14 commits October 15, 2021 22:28

structure, design and exemplar

8510c85

Tweak exemplar and add tests

62bfdfb

No require, return retry tuple, naive datetime

92ce649

format

ce290ed

move to exemplar

cdedf92

Add with concept

fb07c4d

Fix link, cut introduction

076a954

Change birthday to Date

89d039d

trailing white space

b902b6e

Add instructions

7cd27c2

Add hints

bedada4

Tweak design.md

3d0efb8

config.json

3afff39

trailing whitespace

e6de4d7

jiegillet force-pushed the jie-concept-with branch from a801828 to e6de4d7 Compare October 15, 2021 13:28

exercism deleted a comment from github-actions bot Oct 15, 2021

angelikatyborska reviewed Oct 16, 2021

View reviewed changes

jiegillet added 5 commits October 16, 2021 23:57

Add prerequisites

dbd78af

Add contributor

f57f5a6

prerequisites in design.md

154fdf6

Rework instructions

df49629

Rework hints

99bbaa4

jiegillet added 4 commits October 17, 2021 15:40

Rework exemplar

96c474e

Add prerequisites

9cd9331

trailing whitespace

a1fb53b

drop match with =

44c028e

angelikatyborska approved these changes Oct 20, 2021

View reviewed changes

jiegillet and others added 2 commits October 20, 2021 16:37

Apply suggestions from code review

f0072c9

Co-authored-by: Angelika Tyborska <angelikatyborska@fastmail.com>

Cut intro

0baa217

jiegillet merged commit b2fdbbf into exercism:main Oct 20, 2021

jiegillet deleted the jie-concept-with branch October 20, 2021 07:48

jiegillet mentioned this pull request Oct 20, 2021

Implement new Concept Exercise: with #560

Closed

This was referenced Jul 14, 2022

Document reputation label exercism/docs#347

Merged

Consider correct reputation amounts for different labels exercism/exercism#6440

Open

Consider correct reputation amounts for different labels exercism/exercism#6441

Closed

		if time_between(time, ~T[14:00:00], ~T[14:20:00]) do
		{:coffee_break, "information counter staff on coffee break, come back in 15 minutes"}


		## 2. Go to the information desk and find which counter you should go to

		The staff at the information desk is notoriously lazy, they won't do any work during their sacred, excruciatingly long, coffee breaks. If you are lucky enough to find someone on duty, you still need to understand the instructions for which counter to go to. It is so complex that they need to give visitors an instruction manual with the algorithm.


		## 2. Go to the information desk and find which counter you should go to

		The information desk is notorious for taking long coffee breaks. If you are lucky enough to find someone there, they will give you an instruction manual which will explain which counter you need to go to depending on your birth date.

Uh oh!

New concept exercise: new-passport (concept: with) #954

New concept exercise: new-passport (concept: with) #954

Uh oh!

Conversation

jiegillet commented Oct 10, 2021

Uh oh!

jiegillet commented Oct 11, 2021

Uh oh!

angelikatyborska left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jiegillet commented Oct 12, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neenjaw commented Oct 14, 2021

Uh oh!

angelikatyborska commented Oct 14, 2021

Uh oh!

jiegillet commented Oct 15, 2021

Uh oh!

angelikatyborska left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiegillet commented Oct 16, 2021

Uh oh!

github-actions bot commented Oct 17, 2021

Uh oh!

jiegillet commented Oct 17, 2021

Uh oh!

angelikatyborska left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jiegillet commented Oct 20, 2021

Uh oh!

jiegillet commented Oct 20, 2021

Uh oh!

Uh oh!

New concept exercise: `new-passport` (concept: `with`) #954

New concept exercise: `new-passport` (concept: `with`) #954