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

$.uniq doesn't retain order #118

Closed
djones opened this issue Nov 30, 2014 · 9 comments · Fixed by #119
Closed

$.uniq doesn't retain order #118

djones opened this issue Nov 30, 2014 · 9 comments · Fixed by #119

Comments

@djones
Copy link

djones commented Nov 30, 2014

println(",".join($.uniq(["Stockholm", "Stockholm", "SE"])))

Outputs

SE,Stockholm

It should output

Stockholm,SE
@ankurp
Copy link
Owner

ankurp commented Dec 1, 2014

The order is not guaranteed as few things can happen. If order is important then either it is sorted and returned, or it is returned in the order it was passed.

@djones
Copy link
Author

djones commented Dec 1, 2014

it is returned in the order it was passed.

This is what I would expect to happen but that doesn't currently happen.

Order isn't kept for $.uniq but other methods such as $.compact do return things in the order they are passed. So I guess I was expecting $.uniq to do the same.

If you think $.uniq shouldn't keep the order then this issue becomes a feature request for a method that provides $.uniq returning items in the same order they were provided.

@djones
Copy link
Author

djones commented Dec 1, 2014

Also thanks for a great library. It covers off so many things that should be standard in Swift.

@ankurp
Copy link
Owner

ankurp commented Dec 1, 2014

Yes adding it as an option makes sense as underneath it is using dictionary keys and returning that which are uniq but the order is ignored for speed.

@ankurp
Copy link
Owner

ankurp commented Dec 1, 2014

Now the other thing to figure out is what is the order if there are duplicate values as such [Stockholm, SE, Stockholm]

@ankurp
Copy link
Owner

ankurp commented Dec 1, 2014

Should it be [Stockholm, SE] or [SE, Stockholm]. The question is should we pick the last uniq value and its order or pick the first and use that order.

@djones
Copy link
Author

djones commented Dec 1, 2014

I think [Stockholm, SE]. It makes sense to me to iterate through the array in order, accepting return values if they are unique and then discarding anything that comes up that isn't uniq.

So $.uniq([1,2,3,1,4]) outputs [1,2,3,4]

You were talking about performance which is great. Would the optional be something like this:

$.uniq([1,2,3,1,4], preserveOrder: true)

The question is would order preservation be turned on or off by default? I'd be interested to know what you think, but my opinion is it would be better to keep Dollar.swift accurate by default, not tripping people up but have the option of disabling order preservation for speed.

Array's in Swift by default preserve order and so this is why I raised this issue because I ran my array through $.uniq and suddenly it was not in order!

Are there other instances where you've had to decide on speed vs accuracy? If so which did you pick?

@djones
Copy link
Author

djones commented Dec 3, 2014

Thanks heaps @ankurp!

@ankurp
Copy link
Owner

ankurp commented Dec 3, 2014

No problem.

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 a pull request may close this issue.

2 participants