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

peek() returns a tuple instead of a key #12319

Closed
vixsomnis opened this issue Jul 27, 2015 · 7 comments
Closed

peek() returns a tuple instead of a key #12319

vixsomnis opened this issue Jul 27, 2015 · 7 comments
Labels
docs This change adds or pertains to documentation

Comments

@vixsomnis
Copy link

Arch Linux Julia package:
Julia Version 0.3.10
Commit c8ceeef (2015-06-24 13:54 UTC)

(I believe this issue is also present in the latest commit and docs.)

Instead of returning only the key, peek() returns a tuple consisting of a (k,v) tuple.

Is the documentation wrong or is the behavior wrong?

@ivarne
Copy link
Member

ivarne commented Jul 27, 2015

The return type was just changed from Tuple to Pair in #12327, so I assume this should be considered a documentation issue.

@ivarne ivarne added the docs This change adds or pertains to documentation label Jul 27, 2015
@vixsomnis
Copy link
Author

A pair is still essentially two values, however -- which is the conflict I am referencing. Should peek() return a single value in the same way as dequeue!()?

@JeffBezanson
Copy link
Member

@dcjones @kmsquire Was the intent to return just the key or the pair?

@kmsquire
Copy link
Member

When @dcjones created it in 39b5c3d#diff-10b4d49367d18d7903690d5b40f848fcR184, it only returned a key.

@timholy changed it (back?) to a pair in #8011, and that seems to be the intent. I also believe he uses this code, so he should probably chime in.

(I don't think I ever touched this code.)

@timholy
Copy link
Member

timholy commented Jul 28, 2015

Small correction: it always returned the (K,V) tuple. In @dcjones version, xs was an array of tuples, so returning the first element returned a (K,V) pair. For performance reasons (back before tuples were really fast), I changed xs to an array of pairs, and rewrote peek return a tuple for backwards compatibility.

@vixsomnis
Copy link
Author

This is really more about whether peek should return a single value (like how dequeue! returns just the key) or both the key and the value associated with it.

As it stands right now, peek has no real mutating alternative. dequeue! only returns the key, but you can't get the value for that after dequeuing since the priority queue no longer contains the key.

k = dequeue!(pq)
v = pq[k] # doesn't work
dostuff

We can work around this with peek.

kv = peek(pq)
dostuff
dequeue!(pq) # but we don't need the return value anymore

The intent of the second example is obscured by the use of peek (non-mutating) and throwing away the return value of dequeue!.

Should peek return only K? Should dequeue! return (K,V)?

Another possibility is to give pop! a method to handle priority queues that returns a (K,V) the same way peek does, thus keeping dequeue! consistent with enqueue!.

@KristofferC
Copy link
Member

I think this can be closed since peek and corresponding data structure has been moved to https://github.com/JuliaCollections/DataStructures.jl?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

6 participants