-
Notifications
You must be signed in to change notification settings - Fork 35
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
Make Store.items a dequeue #56
Comments
Sorry for the late answer ... SimJulia had a low priority but it will be better in the future;) |
The problem with |
What is the path forward here? |
Boxing immutable objects or using a Dict with as key the object and as value the number of times it is stored. Both are doable. I have a slight preference for the latter. All input is welcome! |
I definitely vote for the latter as well. I'm not a fan of boxing and using a dictionary would make it clear how many items you have. |
I had some spare time;) straightforward implementation of a Store based on a Dict is in the master branch. julia> using SimJulia
julia> env = Simulation()
Simulation time: 0.0 active_process: nothing
julia> store = Store{Symbol}(env)
Store{Symbol}
julia> put(store, :apple)
SimJulia.Put 1
julia> put(store, :apple)
SimJulia.Put 2
julia> put(store, :pear)
SimJulia.Put 3
julia> store.items
Dict{Symbol, UInt64} with 2 entries:
:pear => 0x0000000000000001
:apple => 0x0000000000000002 What do you think? |
Looks great. I've started using this already. Unless you think it needs further work, I think we can close this issue. |
I've been using this new implementation and it works great for my purposes. Unless @riccardosven suggests otherwise, we should close this issue. |
I agree! Closed |
Is there a specific reason why
Store.items
is aSet{T}
?In my opinion it makes for some counterintuitive behavior. For instance,
gives
which is counterintuitive as one would expect the store to contain two apples now; however, before making a pr, I wanted to know if there is some specific idea behind using
Set
.The text was updated successfully, but these errors were encountered: