Miror clarifications to multi-arg example #84
Conversation
|
This should be using JsonDecodeError (as is used with the latest version) |
|
Ah. Assumed this would be in the latest package set. Override for reference: |
|
It’s about to be in the latest package set, but it isn’t yet. |
|
I’m not sure about changing to use Boolean instead of the three constructors; it seems like it introduces Boolean blindness here. What was your reasoning for the change? |
|
To address boolean blindness, would it be clearer to use When reading the original type, I kept misinterpreting who was doing the following (even with the comments). The way it read to me is that the |
README.md
Outdated
| | author == username -> Right You | ||
| -- user is not the author, or no one is logged in, so use the `following` flag | ||
| | otherwise -> Right $ Other author following | ||
| Nothing -> Left $ Named "Username" MissingValue |
There was a problem hiding this comment.
I don't know if Named is the right error to use here. It certainly doesn't seem like AtKey is correct either.
There was a problem hiding this comment.
This isn't quite right; what this example is (supposed to be) getting at is that the Maybe Username argument represents the logged-in user, if there is one.
If it's a Just then someone is logged in, so we check to see if the logged-in person is the author; if they are the same, then we can return You. If they are not the same, then we can return Other and check the following flag.
If it's a Nothing then no one is logged in. In that case the viewer of the article isn't following the author of the article (because they can't be). So following should be false, and we can set the return value to Other author false.
None of these cases should produce an error.
Maybe this could be cleaned up to better represent the idea that you can have your decoding function depend on an argument without getting too bogged down in the details.
There was a problem hiding this comment.
I think the point of confusion for me (beyond the initial package mismatch build errors) is that otherwise was used outside of the guard.
I'll make another PR to restore this as close as possible to the original example.
milesfrain
changed the title
README.md
Outdated
| @@ -440,7 +440,7 @@ type BlogPost = | |||
| } | |||
| ``` | |||
|
|
|||
| Our API tells us the author of the blog post as a string and whether we follow them as a boolean. This admits more cases than are actually possible -- you can't follow yourself, for example -- so we are more precise and model an `Author` as a sum type. | |||
| Our API tells us the author of the blog post is a `String` and whether we follow them is a `Boolean`. This admits more cases than are actually possible -- you can't follow yourself, for example -- so we are more precise and model an `Author` as a sum type. | |||
There was a problem hiding this comment.
I can see the original phrasing making more sense if "tells" is replaced with "sends".
There was a problem hiding this comment.
Yes, "sends" is a better word here.
|
A bit of a goose chase with this one. Should be good now. |
What does this pull request do?
Fixes broken example so it compiles.
Also adds some simplifications without detracting from what this section teaches.
Other Notes:
There are lots of other required fixes in these docs to change
JsonDecodeErrortoString, but that should probably be another PR.A bonus goal is to ensure inline snippets always compile by linking them to source files that are compiled as part of CI.