purescript-contrib/purescript-argonaut-codecs

Deprecate .:? (getFieldOptional) and .!= (defaultField)

thomashoneyman opened this issue · 5 comments

After speaking with @davezuch I realized (at his prompting) that the functions and operators for .:? and .!= are unnecessary and can be replaced without losing any functionality in this library.

For example, given this type:

newtype User = User
  { name :: String 
  , age :: Maybe Int
  , team :: String
  }

derive instance newtypeUser :: Newtype User _
derive newtype instance showUser :: Show User 

these two instances are identical:

instance decodeJsonUser :: DecodeJson User where
  decodeJson json = do
    obj <- decodeJson json
    name <- obj .: "name"
    age <- obj .:? "age"
    team <- obj .:? "team" .!= "Red Team"
    pure $ User { name, age, team }
    
instance decodeJsonUser :: DecodeJson User where
  decodeJson json = do
    obj <- decodeJson json
    name <- obj .: "name"
    age <- obj .: "age"
    team <- obj .: "team" <|> pure "Red Team"
    pure $ User { name, age, team }

Because the Maybe instance for .: already covers the case covered by .:?, and the functionality of .!= (providing a default value for a type which may not exist in the Json, but must exist in the decoded type) is covered by the use of Alternative: <|> pure default. In fact, this is even better, because it introduces to folks the ability to have fallbacks via Alternative rather than lead them to use an overly-specific operator.

> decodeJson =<< jsonParser """{ "name": "Tom", "age": 55, "team": "Blue Team" }""" :: Either String User
(Right { age: (Just 55), name: "Tom", team: "Blue Team" })

> decodeJson =<< jsonParser """{ "name": "Tom", "age": 55 }""" :: Either String User
(Right { age: (Just 55), name: "Tom", team: "Red Team" })

> decodeJson =<< jsonParser """{ "name": "Tom", "age": null }""" :: Either String User
(Right { age: Nothing, name: "Tom", team: "Red Team" })

Side note: I'm not sure that .:! needs to exist either. Its sole difference from .: applied to Maybe is that it will error on a key present with a null value instead of decoding to Nothing. I'm not sure when that would ever be the desired behavior. But that's a question for another day.

I'd like to open a PR which deprecates .:? and .!= and standardizes on just .: and <|> pure default.

Looks good to me, I didn't even realize that .: already works for Maybe types. 👍

I'm all for this! It'll simplify the API and as you said, better to introduce beginners to using fallbacks via Alternative than to lead them towards an unnecessarily specific solution.

From #64

Oh, no: should have looked more closely, but the Maybe instance is not the same as .:?, and it isn't the same as .:!, either. Instead, here's the situation:

  1. The Maybe instance tries to decode a given value using its DecodeJson instance, so long as that value is not null
instance decodeJsonMaybe :: DecodeJson a => DecodeJson (Maybe a) where
  decodeJson j
    | isNull j = pure Nothing
    | otherwise = Just <$> decodeJson j
  1. .: tries to look up a key in an object, failing with a decode error if the key is absent. If the key is present, then it tries to decode the value. If decoding to a Maybe type, then this will use the same null check as the Maybe instance, which makes this similar to .:?.
getField :: forall a. DecodeJson a => FO.Object Json -> String -> Either String a
getField o s =
  maybe
    (Left $ "Expected field " <> show s)
    (elaborateFailure s <<< decodeJson)
    (FO.lookup s o)

infix 7 getField as .:
  1. .:! tries to look up a key in an object, and if it can find that key, it tries to decode the value within by using its DecodeJson instance. Notably, because this is not using the Maybe instance for DecodeJson, this will fail on null where the Maybe instance will not.
getFieldOptional :: forall a. DecodeJson a => FO.Object Json -> String -> Either String (Maybe a)
getFieldOptional o s =
  maybe
    (pure Nothing)
    decode
    (FO.lookup s o)
  where
    decode json = Just <$> (elaborateFailure s <<< decodeJson) json

infix 7 getFieldOptional as .:!
  1. .:? tries to look up a key in an object. If it can find it, then it checks whether its value is null (like the Maybe instance does) and only if it is non-null does it try to decode the value.
getFieldOptional' :: forall a. DecodeJson a => FO.Object Json -> String -> Either String (Maybe a)
getFieldOptional' o s =
  maybe
    (pure Nothing)
    decode
    (FO.lookup s o)
  where
    decode json =
      if isNull json
        then pure Nothing
        else Just <$> (elaborateFailure s <<< decodeJson) json

infix 7 getFieldOptional' as .:?

So, in summary, we have a number of cases:

  1. If the key must be present in the object and its value must exist, then .: works
  2. If the key must be present in the object, but the decoded type is optional, then .: with its Maybe instance works (null becomes Nothing).
  3. If the key is optional, and the value is optional (null == Nothing rather than a decode error), then .:? works
  4. If the key is optional, but the value cannot be null, then .:! works

Further confusing the issue: if you are using .: then you can use Alternative's <|> to provide a fallback in case decoding fails. That means any of:

a) the key is missing
b) the value is null
c) the value could not be decoded

...but you can't recreate the case where you want decoding to fail if the value does not satisfy the decoder, but to decode to Nothing if the key is absent.

So it doesn't actually look like these deprecations make sense.

Whether or not this leads to any changes in the library, this at least deserves documentation.

With the documentation updates and now that it's clear that deprecating .:? and .!= would be incorrect, I'll go ahead and close this.