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

Should Dict.keys return a Set? #317

Closed
rtfeldman opened this Issue Jul 31, 2015 · 8 comments

Comments

Projects
None yet
5 participants
@rtfeldman
Member

rtfeldman commented Jul 31, 2015

Dict.keys has this signature:

keys : Dict comparable v -> List comparable

It seems like Set would make more sense for the return type than List, as the keys in a Dict are:

  1. unique
  2. not ordered

...whereas the elements in a List are ordered and not necessarily unique.

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jul 31, 2015

Member

It's this way based on what I was used to in Haskell, so not the best reasoning.

Best argument I can come up with is that typically I want the keys as a list if I am asking for them. Probably should look at uses of this function in the wild and see how many would be better if it was a set.

Did this come up for you in a particular scenario?

Member

evancz commented Jul 31, 2015

It's this way based on what I was used to in Haskell, so not the best reasoning.

Best argument I can come up with is that typically I want the keys as a list if I am asking for them. Probably should look at uses of this function in the wild and see how many would be better if it was a set.

Did this come up for you in a particular scenario?

@rtfeldman

This comment has been minimized.

Show comment
Hide comment
@rtfeldman

rtfeldman Jul 31, 2015

Member

Yeah, I have a couple of Dicts that map database IDs to records with info used in rendering. The server just wants the ids, not the records, so I use Dict.keys to get them during serialization.

Since I have to give the server a list of unique ids, I end up calling Dict.keys, then later Set.fromList to guarantee uniqueness, then later List.fromSet so I can pass the ids to Json.Encode.

It's barely an inconvenience, but it surprised me because I'm used to Java, which only offers the Set option - and which seems more semantically appropriate.

Member

rtfeldman commented Jul 31, 2015

Yeah, I have a couple of Dicts that map database IDs to records with info used in rendering. The server just wants the ids, not the records, so I use Dict.keys to get them during serialization.

Since I have to give the server a list of unique ids, I end up calling Dict.keys, then later Set.fromList to guarantee uniqueness, then later List.fromSet so I can pass the ids to Json.Encode.

It's barely an inconvenience, but it surprised me because I'm used to Java, which only offers the Set option - and which seems more semantically appropriate.

@TheSeamau5

This comment has been minimized.

Show comment
Hide comment
@TheSeamau5

TheSeamau5 Jul 31, 2015

Contributor

Maybe there should be a choice. The default Dict.keys would return a Set emphasizing the uniqueness nature of the keys whereas Dict.keysAsList would return a list of keys without having to call Set.toList.

Contributor

TheSeamau5 commented Jul 31, 2015

Maybe there should be a choice. The default Dict.keys would return a Set emphasizing the uniqueness nature of the keys whereas Dict.keysAsList would return a list of keys without having to call Set.toList.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 1, 2015

Contributor

Well, Dict.keysAsList (or actually the current Dict.keys) is just Dict.toList >> List.map fst.

I'm with Richard here, that the Dict.keys function would more semantically meaningfully return a set.

And if one wants a list, that functionality is already there (with Dict.toList).

Contributor

jvoigtlaender commented Aug 1, 2015

Well, Dict.keysAsList (or actually the current Dict.keys) is just Dict.toList >> List.map fst.

I'm with Richard here, that the Dict.keys function would more semantically meaningfully return a set.

And if one wants a list, that functionality is already there (with Dict.toList).

@evancz evancz added revisit and removed revisit labels Aug 2, 2015

@evancz evancz referenced this issue Aug 2, 2015

Open

API Ideas #322

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Aug 2, 2015

Member

So I think this idea makes sense, but I'd want to do a comprehensive overhaul of core APIs all at once. We are starting to see some ideas of how we can break things up or make the core data structures work together better, so I'd like to aggregate all those ideas and sort out how to combine them.

I have been talking to a bunch of folks who maintain open source projects and learned some tricks about closing issues more efficiently, even when it's a good idea that we should revisit at a later time. So I want to start closing issues like this and moving the most promising ones to #322 so we can do a comprehensive review when it's time.

Member

evancz commented Aug 2, 2015

So I think this idea makes sense, but I'd want to do a comprehensive overhaul of core APIs all at once. We are starting to see some ideas of how we can break things up or make the core data structures work together better, so I'd like to aggregate all those ideas and sort out how to combine them.

I have been talking to a bunch of folks who maintain open source projects and learned some tricks about closing issues more efficiently, even when it's a good idea that we should revisit at a later time. So I want to start closing issues like this and moving the most promising ones to #322 so we can do a comprehensive review when it's time.

@evancz evancz closed this Aug 2, 2015

@rtfeldman

This comment has been minimized.

Show comment
Hide comment
@rtfeldman

rtfeldman Aug 2, 2015

Member

Love it! 👍

Member

rtfeldman commented Aug 2, 2015

Love it! 👍

@mgold

This comment has been minimized.

Show comment
Hide comment
@mgold

mgold Nov 8, 2015

Contributor

So I know I'm late to this party, but I was thinking: since Dicts and Sets have the same representation except that a set never looks at the values, would it work to just hand the same dict object over and say it's a set? That way this function could run in O(1) rather than O(n) time. I don't think this is possible to implement because Dicts are actual Elm, not a native library, but in theory...

Contributor

mgold commented Nov 8, 2015

So I know I'm late to this party, but I was thinking: since Dicts and Sets have the same representation except that a set never looks at the values, would it work to just hand the same dict object over and say it's a set? That way this function could run in O(1) rather than O(n) time. I don't think this is possible to implement because Dicts are actual Elm, not a native library, but in theory...

@rtfeldman

This comment has been minimized.

Show comment
Hide comment
@rtfeldman

rtfeldman Nov 8, 2015

Member

Bonus! :D

On Sun, Nov 8, 2015, 9:29 PM Max Goldstein notifications@github.com wrote:

So I know I'm late to this party, but I was thinking: since Dicts and Sets
have the same representation except that a set never looks at the values,
would it work to just hand the same dict object over and say it's a set?
That way this function could run in O(1) rather than O(n) time. I don't
think this is possible to implement because Dicts are actual Elm, not a
native library, but in theory...


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/issues/317#issuecomment-154867945.

Member

rtfeldman commented Nov 8, 2015

Bonus! :D

On Sun, Nov 8, 2015, 9:29 PM Max Goldstein notifications@github.com wrote:

So I know I'm late to this party, but I was thinking: since Dicts and Sets
have the same representation except that a set never looks at the values,
would it work to just hand the same dict object over and say it's a set?
That way this function could run in O(1) rather than O(n) time. I don't
think this is possible to implement because Dicts are actual Elm, not a
native library, but in theory...


Reply to this email directly or view it on GitHub
https://github.com/elm-lang/core/issues/317#issuecomment-154867945.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment