* Consider using `Text` instead of `String`, as `String` is a linked-list of `Char` and quite inefficient
* Consider refining some of your fields into more constrained types. Sort orders probably only have a couple of valid values, and might reduce to an enumeration instead of `String`.
* In `instance ToQueryString PriceRequest`, I'd be surprised if those type annotations are necessary. The inferencer should be able to work out that `req` is a `PriceRequest`.
* In `instance FromJSON AggregatedSnapshot`, I think the `read`s should go. I think you want to use the `Float` instance of `FromJSON` instead. (What the `read <$> ...` is doing is parsing to a string, and then using the partial (!) `read` function on the result. JSON responses that have a value that fails to read for that key will crash the program.)
* In `instance FromJSON AggregatedSnapshot`, consider `withObject` here too, which encapsulates the pattern you're using.
I don't have it in front of me but I would assume the FromJSON instance for Float expects a JSON number, not a string. If the remote API makes the (puzzling) decision to put numeric values in strings, then parsing those will be necessary. Although, I agree that the 'read' function is the wrong one to use here, due to it making the function partial (consider 'readMaybe' instead?)
Thank you for all of these suggestions, very appreciated! I'll work through implementing these. Also, please see my explanation of the `read` calls below, but I plan to change those to `readMaybe`s, since there is a need to parse numbers from strings ( for whatever reason :/ )
What a bizarre API. You might get some mileage out of writing a helper function like `readParser :: Read a => String -> Parser a` which can call `readMaybe` and inspect the result.
* Consider refining some of your fields into more constrained types. Sort orders probably only have a couple of valid values, and might reduce to an enumeration instead of `String`.
* In `instance ToQueryString PriceRequest`, I'd be surprised if those type annotations are necessary. The inferencer should be able to work out that `req` is a `PriceRequest`.
* In `instance FromJSON AggregatedSnapshot`, I think the `read`s should go. I think you want to use the `Float` instance of `FromJSON` instead. (What the `read <$> ...` is doing is parsing to a string, and then using the partial (!) `read` function on the result. JSON responses that have a value that fails to read for that key will crash the program.)
* In `instance FromJSON AggregatedSnapshot`, consider `withObject` here too, which encapsulates the pattern you're using.