From 493b5681d42d47f63a261275bb6684e53bc78d00 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 13 Aug 2018 05:04:35 -0400 Subject: [PATCH] Swap part1 and part2 --- advanced/part1/README.md | 2 +- advanced/part1/src/Article.elm | 81 +++++++++++++++++++++------- advanced/part1/src/Author.elm | 4 +- advanced/part1/src/Page.elm | 4 +- advanced/part1/src/Page/Profile.elm | 4 +- advanced/part1/src/Page/Settings.elm | 2 +- advanced/part1/src/Viewer.elm | 2 +- advanced/part1/src/Viewer/Cred.elm | 57 ++++++-------------- advanced/part2/README.md | 2 +- advanced/part2/src/Article.elm | 81 +++++++--------------------- advanced/part2/src/Author.elm | 4 +- advanced/part2/src/Page.elm | 4 +- advanced/part2/src/Page/Profile.elm | 4 +- advanced/part2/src/Page/Settings.elm | 2 +- advanced/part2/src/Viewer.elm | 2 +- advanced/part2/src/Viewer/Cred.elm | 57 ++++++++++++++------ 16 files changed, 156 insertions(+), 156 deletions(-) diff --git a/advanced/part1/README.md b/advanced/part1/README.md index 5cb42a2..bb306de 100644 --- a/advanced/part1/README.md +++ b/advanced/part1/README.md @@ -10,4 +10,4 @@ Then open [http://localhost:3000](http://localhost:3000) in your browser. ## Exercise -Open `src/Article.elm` in your editor and resolve the TODOs there. +Open `src/Viewer/Cred.elm` in your editor and resolve the TODOs there. diff --git a/advanced/part1/src/Article.elm b/advanced/part1/src/Article.elm index ce408ae..b5c550a 100644 --- a/advanced/part1/src/Article.elm +++ b/advanced/part1/src/Article.elm @@ -56,22 +56,64 @@ import Viewer.Cred as Cred exposing (Cred) -- TYPES +{-| An article, optionally with an article body. + +To see the difference between { extraInfo : a } and { extraInfo : Maybe Body }, +consider the difference between the "view individual article" page (which +renders one article, including its body) and the "article feed" - +which displays multiple articles, but without bodies. + +This definition for `Article` means we can write: + +viewArticle : Article Full -> Html msg +viewFeed : List (Article Preview) -> Html msg + +This indicates that `viewArticle` requires an article _with a `body` present_, +wereas `viewFeed` accepts articles with no bodies. (We could also have written +it as `List (Article a)` to specify that feeds can accept either articles that +have `body` present or not. Either work, given that feeds do not attempt to +read the `body` field from articles.) + +This is an important distinction, because in Request.Article, the `feed` +function produces `List (Article Preview)` because the API does not return bodies. +Those articles are useful to the feed, but not to the individual article view. + +-} type Article a = Article Internals a +{-| Metadata about the article - its title, description, and so on. --- 💡 HINT: We can use these `Preview` and/or `Full` types to store information... +Importantly, this module's public API exposes a way to read this metadata, but +not to alter it. This is read-only information! +If we find ourselves using any particular piece of metadata often, +for example `title`, we could expose a convenience function like this: -type Preview - = Preview +Article.title : Article a -> String +If you like, it's totally reasonable to expose a function like that for every one +of these fields! -type Full - = Full - +(Okay, to be completely honest, exposing one function per field is how I prefer +to do it, and that's how I originally wrote this module. However, I'm aware that +this code base has become a common reference point for beginners, and I think it +is _extremely important_ that slapping some "getters and setters" on a record +does not become a habit for anyone who is getting started with Elm. The whole +point of making the Article type opaque is to create guarantees through +_selectively choosing boundaries_ around it. If you aren't selective about +where those boundaries are, and instead expose a "getter and setter" for every +field in the record, the result is an API with no more guarantees than if you'd +exposed the entire record directly! It is so important to me that beginners not +fall into the terrible "getters and setters" trap that I've exposed this +Metadata record instead of exposing a single function for each of its fields, +as I did originally. This record is not a bad way to do it, by any means, +but if this seems at odds with - now you know why! +See commit c2640ae3abd60262cdaafe6adee3f41d84cd85c3 for how it looked before. +) +-} type alias Metadata = { description : String , title : String @@ -89,6 +131,14 @@ type alias Internals = } +type Preview + = Preview + + +type Full + = Full Body + + -- INFO @@ -109,8 +159,8 @@ slug (Article internals _) = body : Article Full -> Body -body _ = - "👉 TODO make this return the article's body" +body (Article _ (Full extraInfo)) = + extraInfo @@ -131,8 +181,8 @@ mapAuthor transform (Article info extras) = fromPreview : Body -> Article Preview -> Article Full -fromPreview _ _ = - "👉 TODO convert from an Article Preview to an Article Full" +fromPreview newBody (Article info Preview) = + Article info (Full newBody) @@ -150,16 +200,7 @@ fullDecoder : Maybe Cred -> Decoder (Article Full) fullDecoder maybeCred = Decode.succeed Article |> custom (internalsDecoder maybeCred) - |> required "body" "👉 TODO use `Body.decoder` (which is a `Decoder Body`) to decode the body into this Article Full" - - - -{- If you're unfamiliar with Decode Pipeline, here's how ☝️ would look without it: - - Decode.map2 Article - (internalsDecoder maybeCred) - (Decode.field "body" "use `Body.decoder` (which is a `Decoder Body`) to decode the body into this Article Full") --} + |> required "body" (Decode.map Full Body.decoder) internalsDecoder : Maybe Cred -> Decoder Internals diff --git a/advanced/part1/src/Author.elm b/advanced/part1/src/Author.elm index 6685cb7..e8a3f91 100644 --- a/advanced/part1/src/Author.elm +++ b/advanced/part1/src/Author.elm @@ -95,7 +95,7 @@ username : Author -> Username username author = case author of IsViewer cred _ -> - Cred.username cred + cred.username IsFollowing (FollowedAuthor val _) -> val @@ -220,7 +220,7 @@ decodeFromPair maybeCred ( prof, uname ) = Decode.succeed (IsNotFollowing (UnfollowedAuthor uname prof)) Just cred -> - if uname == Cred.username cred then + if uname == cred.username then Decode.succeed (IsViewer cred prof) else diff --git a/advanced/part1/src/Page.elm b/advanced/part1/src/Page.elm index 2986b9c..d60758e 100644 --- a/advanced/part1/src/Page.elm +++ b/advanced/part1/src/Page.elm @@ -71,8 +71,8 @@ viewMenu page maybeViewer = cred = Viewer.cred viewer - username = - Cred.username cred + { username } = + cred avatar = Profile.avatar (Viewer.profile viewer) diff --git a/advanced/part1/src/Page/Profile.elm b/advanced/part1/src/Page/Profile.elm index a9d59b1..996095e 100644 --- a/advanced/part1/src/Page/Profile.elm +++ b/advanced/part1/src/Page/Profile.elm @@ -88,7 +88,7 @@ view model = titleForOther (Author.username author) Loading username -> - if Just username == Maybe.map Cred.username (Session.cred model.session) then + if Just username == Maybe.map .username (Session.cred model.session) then myProfileTitle else @@ -96,7 +96,7 @@ view model = Failed username -> -- We can't follow if it hasn't finished loading yet - if Just username == Maybe.map Cred.username (Session.cred model.session) then + if Just username == Maybe.map .username (Session.cred model.session) then myProfileTitle else diff --git a/advanced/part1/src/Page/Settings.elm b/advanced/part1/src/Page/Settings.elm index 87220d7..3bae816 100644 --- a/advanced/part1/src/Page/Settings.elm +++ b/advanced/part1/src/Page/Settings.elm @@ -58,7 +58,7 @@ init session = { avatar = Avatar.toMaybeString (Profile.avatar profile) , email = Email.toString (Viewer.email viewer) , bio = Maybe.withDefault "" (Profile.bio profile) - , username = Username.toString (Cred.username cred) + , username = Username.toString cred.username , password = Nothing } diff --git a/advanced/part1/src/Viewer.elm b/advanced/part1/src/Viewer.elm index f8480c7..a070af8 100644 --- a/advanced/part1/src/Viewer.elm +++ b/advanced/part1/src/Viewer.elm @@ -55,7 +55,7 @@ encode : Viewer -> Value encode (Viewer info) = Encode.object [ ( "email", Email.encode info.email ) - , ( "username", Username.encode (Cred.username info.cred) ) + , ( "username", Username.encode info.cred.username ) , ( "bio", Maybe.withDefault Encode.null (Maybe.map Encode.string (Profile.bio info.profile)) ) , ( "image", Avatar.encode (Profile.avatar info.profile) ) , ( "token", Cred.encodeToken info.cred ) diff --git a/advanced/part1/src/Viewer/Cred.elm b/advanced/part1/src/Viewer/Cred.elm index d010acc..50151f1 100644 --- a/advanced/part1/src/Viewer/Cred.elm +++ b/advanced/part1/src/Viewer/Cred.elm @@ -1,20 +1,4 @@ -module Viewer.Cred exposing (Cred, addHeader, addHeaderIfAvailable, decoder, encodeToken, username) - -{-| The authentication credentials for the Viewer (that is, the currently logged-in user.) - -This includes: - - - The cred's Username - - The cred's authentication token - -By design, there is no way to access the token directly as a String. -It can be encoded for persistence, and it can be added to a header -to a HttpBuilder for a request, but that's it. - -This token should never be rendered to the end user, and with this API, it -can't be! - --} +module Viewer.Cred exposing (Cred, addHeader, addHeaderIfAvailable, decoder, encodeToken) import HttpBuilder exposing (RequestBuilder, withHeader) import Json.Decode as Decode exposing (Decoder) @@ -23,30 +7,21 @@ import Json.Encode as Encode exposing (Value) import Username exposing (Username) -{-| The authentication token for the currently logged-in user. - -The token records the username associated with this token, which you can ask it for. - -By design, there is no way to access the token directly as a String. You can encode it for persistence, and you can add it to a header to a HttpBuilder for a request, but that's it. - --} - - -- TYPES -type Cred - = Cred Username String - - - --- INFO - - -username : Cred -> Username -username (Cred val _) = - val +type alias Cred = + -- 👉 TODO make Cred an opaque type, then fix the resulting compiler errors. + -- Afterwards, it should no longer be possible for any other module to access + -- this `token` vale directly! + -- + -- 💡 HINT: Other modules still depend on being able to access the + -- `username` value. Expand this module's API to expose a new way for them + -- to access the `username` without also giving them access to `token`. + { username : Username + , token : String + } @@ -65,14 +40,14 @@ decoder = encodeToken : Cred -> Value -encodeToken (Cred _ str) = - Encode.string str +encodeToken cred = + Encode.string cred.token addHeader : Cred -> RequestBuilder a -> RequestBuilder a -addHeader (Cred _ str) builder = +addHeader cred builder = builder - |> withHeader "authorization" ("Token " ++ str) + |> withHeader "authorization" ("Token " ++ cred.token) addHeaderIfAvailable : Maybe Cred -> RequestBuilder a -> RequestBuilder a diff --git a/advanced/part2/README.md b/advanced/part2/README.md index fb636c3..d2f880a 100644 --- a/advanced/part2/README.md +++ b/advanced/part2/README.md @@ -10,4 +10,4 @@ Then open [http://localhost:3000](http://localhost:3000) in your browser. ## Exercise -Open `src/Viewer/Cred.elm` in your editor and resolve the TODOs there. +Open `src/Article.elm` in your editor and resolve the TODOs there. diff --git a/advanced/part2/src/Article.elm b/advanced/part2/src/Article.elm index b5c550a..ce408ae 100644 --- a/advanced/part2/src/Article.elm +++ b/advanced/part2/src/Article.elm @@ -56,64 +56,22 @@ import Viewer.Cred as Cred exposing (Cred) -- TYPES -{-| An article, optionally with an article body. - -To see the difference between { extraInfo : a } and { extraInfo : Maybe Body }, -consider the difference between the "view individual article" page (which -renders one article, including its body) and the "article feed" - -which displays multiple articles, but without bodies. - -This definition for `Article` means we can write: - -viewArticle : Article Full -> Html msg -viewFeed : List (Article Preview) -> Html msg - -This indicates that `viewArticle` requires an article _with a `body` present_, -wereas `viewFeed` accepts articles with no bodies. (We could also have written -it as `List (Article a)` to specify that feeds can accept either articles that -have `body` present or not. Either work, given that feeds do not attempt to -read the `body` field from articles.) - -This is an important distinction, because in Request.Article, the `feed` -function produces `List (Article Preview)` because the API does not return bodies. -Those articles are useful to the feed, but not to the individual article view. - --} type Article a = Article Internals a -{-| Metadata about the article - its title, description, and so on. -Importantly, this module's public API exposes a way to read this metadata, but -not to alter it. This is read-only information! +-- 💡 HINT: We can use these `Preview` and/or `Full` types to store information... -If we find ourselves using any particular piece of metadata often, -for example `title`, we could expose a convenience function like this: -Article.title : Article a -> String +type Preview + = Preview -If you like, it's totally reasonable to expose a function like that for every one -of these fields! -(Okay, to be completely honest, exposing one function per field is how I prefer -to do it, and that's how I originally wrote this module. However, I'm aware that -this code base has become a common reference point for beginners, and I think it -is _extremely important_ that slapping some "getters and setters" on a record -does not become a habit for anyone who is getting started with Elm. The whole -point of making the Article type opaque is to create guarantees through -_selectively choosing boundaries_ around it. If you aren't selective about -where those boundaries are, and instead expose a "getter and setter" for every -field in the record, the result is an API with no more guarantees than if you'd -exposed the entire record directly! It is so important to me that beginners not -fall into the terrible "getters and setters" trap that I've exposed this -Metadata record instead of exposing a single function for each of its fields, -as I did originally. This record is not a bad way to do it, by any means, -but if this seems at odds with - now you know why! -See commit c2640ae3abd60262cdaafe6adee3f41d84cd85c3 for how it looked before. -) +type Full + = Full + --} type alias Metadata = { description : String , title : String @@ -131,14 +89,6 @@ type alias Internals = } -type Preview - = Preview - - -type Full - = Full Body - - -- INFO @@ -159,8 +109,8 @@ slug (Article internals _) = body : Article Full -> Body -body (Article _ (Full extraInfo)) = - extraInfo +body _ = + "👉 TODO make this return the article's body" @@ -181,8 +131,8 @@ mapAuthor transform (Article info extras) = fromPreview : Body -> Article Preview -> Article Full -fromPreview newBody (Article info Preview) = - Article info (Full newBody) +fromPreview _ _ = + "👉 TODO convert from an Article Preview to an Article Full" @@ -200,7 +150,16 @@ fullDecoder : Maybe Cred -> Decoder (Article Full) fullDecoder maybeCred = Decode.succeed Article |> custom (internalsDecoder maybeCred) - |> required "body" (Decode.map Full Body.decoder) + |> required "body" "👉 TODO use `Body.decoder` (which is a `Decoder Body`) to decode the body into this Article Full" + + + +{- If you're unfamiliar with Decode Pipeline, here's how ☝️ would look without it: + + Decode.map2 Article + (internalsDecoder maybeCred) + (Decode.field "body" "use `Body.decoder` (which is a `Decoder Body`) to decode the body into this Article Full") +-} internalsDecoder : Maybe Cred -> Decoder Internals diff --git a/advanced/part2/src/Author.elm b/advanced/part2/src/Author.elm index e8a3f91..6685cb7 100644 --- a/advanced/part2/src/Author.elm +++ b/advanced/part2/src/Author.elm @@ -95,7 +95,7 @@ username : Author -> Username username author = case author of IsViewer cred _ -> - cred.username + Cred.username cred IsFollowing (FollowedAuthor val _) -> val @@ -220,7 +220,7 @@ decodeFromPair maybeCred ( prof, uname ) = Decode.succeed (IsNotFollowing (UnfollowedAuthor uname prof)) Just cred -> - if uname == cred.username then + if uname == Cred.username cred then Decode.succeed (IsViewer cred prof) else diff --git a/advanced/part2/src/Page.elm b/advanced/part2/src/Page.elm index d60758e..2986b9c 100644 --- a/advanced/part2/src/Page.elm +++ b/advanced/part2/src/Page.elm @@ -71,8 +71,8 @@ viewMenu page maybeViewer = cred = Viewer.cred viewer - { username } = - cred + username = + Cred.username cred avatar = Profile.avatar (Viewer.profile viewer) diff --git a/advanced/part2/src/Page/Profile.elm b/advanced/part2/src/Page/Profile.elm index 996095e..a9d59b1 100644 --- a/advanced/part2/src/Page/Profile.elm +++ b/advanced/part2/src/Page/Profile.elm @@ -88,7 +88,7 @@ view model = titleForOther (Author.username author) Loading username -> - if Just username == Maybe.map .username (Session.cred model.session) then + if Just username == Maybe.map Cred.username (Session.cred model.session) then myProfileTitle else @@ -96,7 +96,7 @@ view model = Failed username -> -- We can't follow if it hasn't finished loading yet - if Just username == Maybe.map .username (Session.cred model.session) then + if Just username == Maybe.map Cred.username (Session.cred model.session) then myProfileTitle else diff --git a/advanced/part2/src/Page/Settings.elm b/advanced/part2/src/Page/Settings.elm index 3bae816..87220d7 100644 --- a/advanced/part2/src/Page/Settings.elm +++ b/advanced/part2/src/Page/Settings.elm @@ -58,7 +58,7 @@ init session = { avatar = Avatar.toMaybeString (Profile.avatar profile) , email = Email.toString (Viewer.email viewer) , bio = Maybe.withDefault "" (Profile.bio profile) - , username = Username.toString cred.username + , username = Username.toString (Cred.username cred) , password = Nothing } diff --git a/advanced/part2/src/Viewer.elm b/advanced/part2/src/Viewer.elm index a070af8..f8480c7 100644 --- a/advanced/part2/src/Viewer.elm +++ b/advanced/part2/src/Viewer.elm @@ -55,7 +55,7 @@ encode : Viewer -> Value encode (Viewer info) = Encode.object [ ( "email", Email.encode info.email ) - , ( "username", Username.encode info.cred.username ) + , ( "username", Username.encode (Cred.username info.cred) ) , ( "bio", Maybe.withDefault Encode.null (Maybe.map Encode.string (Profile.bio info.profile)) ) , ( "image", Avatar.encode (Profile.avatar info.profile) ) , ( "token", Cred.encodeToken info.cred ) diff --git a/advanced/part2/src/Viewer/Cred.elm b/advanced/part2/src/Viewer/Cred.elm index 50151f1..d010acc 100644 --- a/advanced/part2/src/Viewer/Cred.elm +++ b/advanced/part2/src/Viewer/Cred.elm @@ -1,4 +1,20 @@ -module Viewer.Cred exposing (Cred, addHeader, addHeaderIfAvailable, decoder, encodeToken) +module Viewer.Cred exposing (Cred, addHeader, addHeaderIfAvailable, decoder, encodeToken, username) + +{-| The authentication credentials for the Viewer (that is, the currently logged-in user.) + +This includes: + + - The cred's Username + - The cred's authentication token + +By design, there is no way to access the token directly as a String. +It can be encoded for persistence, and it can be added to a header +to a HttpBuilder for a request, but that's it. + +This token should never be rendered to the end user, and with this API, it +can't be! + +-} import HttpBuilder exposing (RequestBuilder, withHeader) import Json.Decode as Decode exposing (Decoder) @@ -7,21 +23,30 @@ import Json.Encode as Encode exposing (Value) import Username exposing (Username) +{-| The authentication token for the currently logged-in user. + +The token records the username associated with this token, which you can ask it for. + +By design, there is no way to access the token directly as a String. You can encode it for persistence, and you can add it to a header to a HttpBuilder for a request, but that's it. + +-} + + -- TYPES -type alias Cred = - -- 👉 TODO make Cred an opaque type, then fix the resulting compiler errors. - -- Afterwards, it should no longer be possible for any other module to access - -- this `token` vale directly! - -- - -- 💡 HINT: Other modules still depend on being able to access the - -- `username` value. Expand this module's API to expose a new way for them - -- to access the `username` without also giving them access to `token`. - { username : Username - , token : String - } +type Cred + = Cred Username String + + + +-- INFO + + +username : Cred -> Username +username (Cred val _) = + val @@ -40,14 +65,14 @@ decoder = encodeToken : Cred -> Value -encodeToken cred = - Encode.string cred.token +encodeToken (Cred _ str) = + Encode.string str addHeader : Cred -> RequestBuilder a -> RequestBuilder a -addHeader cred builder = +addHeader (Cred _ str) builder = builder - |> withHeader "authorization" ("Token " ++ cred.token) + |> withHeader "authorization" ("Token " ++ str) addHeaderIfAvailable : Maybe Cred -> RequestBuilder a -> RequestBuilder a