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

support Int-keyed Dicts #44

Open
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

CarstenKoenig
Copy link

I had an issues with Int-keyed dicts and this one solves it (basically the same as #37)

BTW: this is the example this PR fixes: https://github.com/CarstenKoenig/YouVote/blob/master/src/server/Poll/Models.hs

@krisajenkins
Copy link
Owner

Thanks for this @CarstenKoenig. I'm just taking a look, and I'm confused by the decoder. Take a look at this Haskell code:

ghci> import Data.Aeson
ghci> import Data.IntMap
ghci> let a = Data.IntMap.fromList [(1, "Alice"), (2, "Bob")]
ghci> a
fromList [(1,"Alice"),(2,"Bob")]
ghci> encode a
"[[1,\"Alice\"],[2,\"Bob\"]]"

...so Aeson encodes an IntMap as a list of pairs. Which doesn't seem to fit with the decoder you've PRd. Thoughts?

@CarstenKoenig
Copy link
Author

CarstenKoenig commented Aug 18, 2017

hmm ... I see your point - and this might be a problem - I used a simple Map (yes of course IntMap would be the better choice) - and with it you get an object:

Prelude> import Data.Aeson
Prelude Data.Aeson> import Data.Map
Prelude Data.Aeson Data.Map> let a = fromList [(1,"Alice"),(2,"Bob")]
Prelude Data.Aeson Data.Map> a
fromList [(1,"Alice"),(2,"Bob")]
Prelude Data.Aeson Data.Map> encode a
"{\"1\":\"Alice\",\"2\":\"Bob\"}"

now I'm not really sure what to do here aside from introducing a different data-constructor (EMap maybe?) - I can try to rewrite the PR for this if you want me to.

On the other hand it might be sensible to tell people (me included) to just use IntMap but IMO as the chances that someone might use tuples or lists of comparable (in Elm) types as keys might be slim anyways

What do you think?


PS: btw - that explains your very sensible test case I destroyed there (sorry) - should have had a more careful look at things

@CarstenKoenig
Copy link
Author

ok - I added basic support for the idea I gave above - it should now tread Map Int smth as a EMap instead of a EDict and encode this as a JSON object - it still works with my custom scenario and it should not change the behavior you initially implemented

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

Successfully merging this pull request may close these issues.

2 participants