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

Seeking advice: SortedDict constructors #781

Closed
StephenVavasis opened this issue Dec 20, 2021 · 2 comments
Closed

Seeking advice: SortedDict constructors #781

StephenVavasis opened this issue Dec 20, 2021 · 2 comments

Comments

@StephenVavasis
Copy link
Contributor

Right now I'm making major revisions to the sorted containers to address several open issues. Here is an issue that I was not planning to address but have stumbled across. The file https://github.com/JuliaCollections/DataStructures.jl/blob/master/src/sorted_dict.jl starts off with a complex sequence of constructors. I'm having trouble understanding the code that attempts to deduce types, and in any case it doesn't work as well as Dict. For example:

julia> w = SortedDict((i=>i^2) for i=1:3)
SortedDict{Any, Any, Base.Order.ForwardOrdering} with 3 entries:
  1 => 1
  2 => 4
  3 => 9

julia> Dict((i=>i^2) for i=1:3)
Dict{Int64, Int64} with 3 entries:
  2 => 4
  3 => 9
  1 => 1

I'm wondering: would it be OK for the constructor SortedDict(kv::Any) to pass kv to collect, and then build the SortedDict from the resulting Vector? The function collect, like Dict, excels at deducing types. The disadvantage is that it is inefficient to copy the data twice, but the documentation can state that the constructor SortedDict{K,V}(kv) is preferred over SortedDict(kv).

@oxinabox
Copy link
Member

oxinabox commented Jan 11, 2022

Yes, that would be OK. We do that in a few places I think.
We can always improve it later.

@StephenVavasis
Copy link
Contributor Author

Closed via #787

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

No branches or pull requests

2 participants