Skip to content

Add 'lookup' function to Data.Set #291

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

DrNico
Copy link

@DrNico DrNico commented Jun 27, 2016

This is an implementation of the proposal submitted as Issue #290, but not yet discussed on the Haskell libraries list.

* WHAT?

This commit contains the implementation of a proposal to add a 'lookup'
function on 'Set' in the "containers" package.

The function

> lookup :: Ord a => a -> Set a -> Maybe a

is almost indentical to the 'member' function but, in addition, returns the value
stored in the set.

* WHY?

The point of this proposal is to facilitate program-wide data sharing. The 'lookup'
function gives access to a pointer to an object already stored in a Set and equal
to a given argument. The 'lookup' function is a natural extension to the current
'lookupLT', 'lookupGT', 'lookupLE' and 'lookupGE' functions, with obvious semantics.

Example use case: In a parser, the memory footprint can be reduced by collapsing
all equal strings to a single instance of each string. To achieve this, one needs
a way to get a previously seen string (internally, a pointer) equal to a newly
parsed string. Amazingly, this is very difficult with the current "containers" library interface. One current option is to use a Map instead, e.g., 'Map String String'
which stores twice as many pointers as necessary.

* HOW?

The git commit contains the straight-forward implementation of the 'lookup' function
on 'Set', with test cases.
@treeowl
Copy link
Contributor

treeowl commented Jun 29, 2016

Unless someone comes up with a better name, or a good reason to dislike this one, I'd like to go with @andreasabel's suggestion of lookupEntry, and to add to Data.Map lookupEntry :: Ord k => k -> Map k v -> Maybe (k, v).

@m-renaud
Copy link
Contributor

Could you also add a similar function to Data.IntSet and Data.IntMap?

@treeowl
Copy link
Contributor

treeowl commented Dec 26, 2017

@m-renaud In the library list discussion, several people thought this was a bad idea. I believe that included @wrengr. They thought this sort of interning operation would be best handled by another data structure. Nothing about this is at all relevant to Data.IntMap or Data.IntSet; they (operationally) store Int# values, not Int values, so there can be no duplication.

@treeowl
Copy link
Contributor

treeowl commented Dec 26, 2017

@m-renaud, sorry, I ascribed the wrong objection to @wrengr. She opposed calling this a name relating to interning. The main dispute on the libraries list had to do with the name. The name lookup seems very strange to me here.

@treeowl
Copy link
Contributor

treeowl commented Dec 26, 2017

The mailing list discussion thread begins with https://mail.haskell.org/pipermail/libraries/2016-June/027116.html

@m-renaud
Copy link
Contributor

Nothing about this is at all relevant to Data.IntMap or Data.IntSet

Ahh, that makes sense.

The main dispute on the libraries list had to do with the name

Gotcha. Reading through the discussion it seems like everyone was okay with lookupEntry as you mention in your comment above. The only hesitation was that for sets the term "entry" doesn't existing in the vocabulary. Should we start a new thread with a proposal with the functions to make sure everyone is on board still?

lookupEntry :: Ord a => a -> Set a -> Maybe a
lookupEntry :: Ord k => k -> Map k v -> Maybe (k, v)

@treeowl
Copy link
Contributor

treeowl commented Dec 26, 2017 via email

@m-renaud
Copy link
Contributor

m-renaud commented Dec 26, 2017

Giving this some more thought, why add lookupEntry for Data.Map? Map already has lookup :: k -> Map k v -> Maybe v, so if you really wanted you could implement Map.lookupEntry k m = (\v -> (k,v)) <$> lookup k m without loss of performance.

Also, since "entry" is really parlance related to maps, removing this from Map means we could use the more appropriate term "elem(ent)": lookupElement :: (Ord a) => a -> Set a -> Maybe a.

WDYT?

@treeowl
Copy link
Contributor

treeowl commented Dec 26, 2017 via email

@m-renaud
Copy link
Contributor

The purpose for maps would be to extract the exact stored key.

Thanks for the clarification; I retract my previous comment. I'll go ahead and send a proposal to libraries for the two lookupEntry methods in #291 (comment).

@sjakobi
Copy link
Member

sjakobi commented Jul 15, 2020

Thanks for the clarification; I retract my previous comment. I'll go ahead and send a proposal to libraries for the two lookupEntry methods in #291 (comment).

Did that libraries proposal ever happen?

@sjakobi sjakobi linked an issue Jul 15, 2020 that may be closed by this pull request
@sjakobi sjakobi marked this pull request as draft July 15, 2020 13:29
@sjakobi sjakobi added Set needs-libraries-proposal A proposal needs to be discussed on the libraries mailing list labels Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request needs-libraries-proposal A proposal needs to be discussed on the libraries mailing list Set
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add lookup function to Data.Set
4 participants