Haskell-Decimal
Haskell-Decimal copied to clipboard
PaulJohnson
Consider adding Generic and Data instances to Decimal
These are useful for users of your code to avoid writing orphan instances and they can be derived by GHC.
OK, but I've never used either of those features. Can you submit a pull request? I know its probably trivial, but I'd rather it be done by someone who knows what the Right Thing is.
There was a bit of discussion related to this request a couple of years ago here.
I don’t recall why I didn’t suggest adding a Generic instance at the time, but it was probably simply that it requires adding a dependency on GHC that many library authors at the time didn’t like requiring.
That was mostly about JSON instances. The issue there was that an auto-generated JSON instance for Decimal doesn't do what people naturally want, which is to write or parse a JSON number.
I think the solution is to put in the Generic and Data instances, and then document the issues with JSON. The native representation of JSON numbers in aeson is Scientific. The simplest way to convert a Scientific to a Decimal is (fromRational . toRational). You can also use "realFracToDecimal" to specify the precision you want, which would be the Right Thing for financial values.
Creating some synonyms for these two functions specialised for Scientific would be useful to avoid the need for ":: Scientific" to be scattered around JSON instances. That would mean adding a dependency on Scientific, but that's much less of an issue than bringing in aeson.
Alternatively this could be done in a separate package which doesn't declare instances but instead provides the toolkit for writing idomatic JSON instances for records that include Decimal values. That would probably be the best solution.
Yes, I meant that the PR related to that issue included the change that made it possible to derive generic instances of all types in the Data.Decimal package. GHC.Generics and Data.Data are in base, so I don’t see any reason not to include the instances in the package.
I agree with your comments on documenting the issues with implementing Aeson instances.
Sorry, I can't find the PR you are referring to. The only one I could find was the removal of an Integral constraint, which I applied back in 2014.
Yes, that’s what I’m referring to. GHC can’t derive generic instances for datatypes with class constraints, which is why I suggested removing the constraint in 2014.
Sorry for the confusion, I was just trying to note that we’d done some work in this direction a long time ago, and I was trying to recall why we hadn’t added a Generic instance to the library at the time. I think the only reason it didn’t get added then is that I wanted to propose as minimal a change as possible; I now do think it makes sense to include the instance in the library, and I don’t think there’s any tradeoff in doing so.
OK, so if you want to put in a PR for the new instances then I'll accept it.