jcpetruzza / barbies

BSD 3-Clause "New" or "Revised" License
92 stars 15 forks source link

Add bmapC to allow mapping with constraints #9

Closed ChrisPenner closed 5 years ago

ChrisPenner commented 5 years ago

Hey there! I've been intrigued by the possibility of using HKD for form libraries, json deserialization, etc. For most of these things though I need to be able to make assumptions about the underlying data using constraints. I think most of the combinators could probably benefit from a constraint scoped version, but figured I'd start off with bmapC.

I'm not sure how you prefer to add tests for these sorts of things, but noticed you didn't have tests for buniqC so I figured I'd just ask 😄

Motivation:

Here's just one example where I may need to assert that all members of the Record have FromJSON in my bMap; this example lets me deserialize each field or error by using a record full of aeson keys.

data User f = User
  { userId :: f String
  , country :: f String
  , interests :: f [String]
  , age :: f Int
  } deriving ( Generic
             , FunctorB, TraversableB, ProductB, ConstraintsB, ProductBC
             )

deriving instance (forall a. Show a => Show (f a)) => Show (User f)

jsonKeys :: User (Const T.Text)
jsonKeys = User
  { userId    = Const "user_id"
  , country   = Const "country"
  , interests = Const "interests"
  , age       = Const "age"
  }

userFromJson :: User (ReaderT Value Maybe)
userFromJson = bmapC @FromJSON (atKey . getConst) jsonKeys
 where
  atKey :: FromJSON a => T.Text -> ReaderT Value Maybe a
  atKey k = do
    v <- ask
    lift (preview (key k) v >>= fromResult . fromJSON)

fromResult :: Result a -> Maybe a
fromResult (Success a) = Just a
fromResult _           = Nothing
jcpetruzza commented 5 years ago

Thanks for submitting this! I agree that something like bmapC will be valuable in terms of usability without bloating the library too much. I think the proposed type for bmapC may be a bit too restrictive, though. I'd expect instead something like this:

bmapC
  :: forall c b f g
  . (AllB c b, ConstraintsB b)
  => (forall a. c a => f a -> g a)
  -> b f -> b g

In particular, this would make bmapC usable not only with products, like in your motivating example, but with sum types as well. Intuitively, one should be able to use baddDicts to enrich the b f with the needed dictionaries and then just bmap the uncurried version of the given function, or something along those lines :slightly_smiling_face:

Regarding tests, I think that for bmapC, once you fix the type, there is essentially only one function you can write with that type, apart from obviously broken things like from bmapC = undefined, bmapC = bmapC, bmapC _ = bmap undefined, etc. So while a test here wouldn't be as crucial as the tests for the generic derivations imo, any test that ensures that a bottom implementation isn't accidentally merged in would be nice to have (something like bmapC @Show (const ()) (Record1 True) == Record1 () would do). Same thing applies to buniqC, of course, where it seems I was lazy :smile:

Unrelated to the PR, the json example you mention is a very good use-case. In your example, you probably want userFromJson to have type ReaderT Value (User Maybe),though, for which you'd need to add a bsequence at the top-level. And this can then be probably generalized to something like:

type Label = Text
bfromJson
  :: (TraversableB b, ConstraintsB b, AllB FromJSON b)
  => b (Const Label)
  -> Value -> b (Either Text) 

So you get a function that only needs the labels you expect for each field and also allow for some error message on fields that didn't parse (I've been meaning to put together a package implementing something like this, but haven't done any json processing lately so it never happened :smile:)

ChrisPenner commented 5 years ago

Sounds good; Relaxing the type makes sense too, I'll try out what you suggest (and add some sort of test) when I get a minute 😄

I'm experimenting a bit with using HKDs for JSON stuff, for validation, and for maybe even some web-form use-cases too! There are tons of possible uses here 😄

ChrisPenner commented 5 years ago

You were right; implementation was easy-peasy; I'll add a test tomorrow probably 😄

ChrisPenner commented 5 years ago

@jcpetruzza Should be good to go now!

I threw in a test case for buniqC while I was at it 😋

jcpetruzza commented 5 years ago

Thanks!