Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why did fromObject change? #30

Open
vikraman opened this issue Nov 24, 2016 · 3 comments
Open

Why did fromObject change? #30

vikraman opened this issue Nov 24, 2016 · 3 comments

Comments

@vikraman
Copy link

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
Copy link
Member

iphydf commented Nov 25, 2016

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
Copy link

SX91 commented Nov 25, 2016

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

@devwout
Copy link

devwout commented May 9, 2018

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants