TokTok / hs-msgpack-binary

Haskell implementation of MessagePack / msgpack.org
http://msgpack.org/
Other
16 stars 10 forks source link

Why did fromObject change? #30

Open vikraman opened 7 years ago

vikraman commented 7 years ago

I was trying to update some old code which was still using the msgpack package, so I tried switching to this fork. I see the MessagePack class now has

fromObject :: (Applicative m, Monad m) => Object -> m a

instead of

fromObject :: Object -> Maybe a

Can you explain the rationale for this change? I see no reason why fromObject needs to work polymorphically over all monads, it has nothing to do with monads at all! You end up using just the fail method for all your instances. At best, you could change Maybe a to Either Error a using an Error datatype.

iphydf commented 7 years ago

Using Either seems reasonable. What I really want here is MonadFail, but that's currently not portable to all the targets I want to support. fromObject does have a lot to do with monads, though, and the polymorphic definition is mostly compatible with code that uses Maybe, while changing it to another concrete instance of Monad is incompatible.

Why do you say it has nothing to do with monads? For the simplest cases, it requires at least Functor, and for slightly more complex cases (product types), it will need Applicative. In general, it's entirely reasonable to need a Monad for decoding values, e.g. when decoding sum types: you need to first decode the object to (Int, Object), use the Int to select the constructor, and then decode the Object to its arguments.

You're arguing for using a specific instance of Monad rather than abstracting over arbitrary ones. I changed this, because I wanted to allow users to run fromObject in an arbitrary monad, such as Data.Binary.Get. I can be convinced to use Either, instead, but it would be an incompatible change.

Why do you feel this is a problem? Are you running into performance issues due to the abstraction?

SX91 commented 7 years ago

Do not change please. It's perfectly fine the way it is now.

devwout commented 6 years ago

The problem I am experiencing with this change, unless I am missing something fundamental about Haskell, is that it is impossible to handle failure in a generic way.

When fromObject returned a Maybe, it was trivial to detect and handle failures, so you could write something like this:

newtype Wrapper a = Wrapper a

instance MessagePack a => MessagePack (Wrapper a) where
  fromObject (Wrapper a) = msum [fromObject [a], fromObject a]

I don't think you can do the equivalent with the new implementation, as failure may have a completely different manifestation depending on the monad?