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

C5.HashDictionary<K, int>.Values are returned on first call only #41

Closed
ddur opened this issue Feb 18, 2016 · 8 comments
Closed

C5.HashDictionary<K, int>.Values are returned on first call only #41

ddur opened this issue Feb 18, 2016 · 8 comments
Labels
bug Something isn't working

Comments

@ddur
Copy link

ddur commented Feb 18, 2016

C5 2.4.5891.40110 (NuGet)
Net 4.5.2

@ddur ddur changed the title C5.HashDictionary<K, T>.Values are returned on first call only C5.HashDictionary<K, V>.Values are returned on first call only Feb 18, 2016
@ddur ddur changed the title C5.HashDictionary<K, V>.Values are returned on first call only C5.HashDictionary<K, int>.Values are returned on first call only Feb 18, 2016
@ddur
Copy link
Author

ddur commented Feb 18, 2016

To be precise:
I use C5.HashDictionary<K, int>.Values
On second call to Values, all int values are 0 (zero)

@ondfisk
Copy link
Collaborator

ondfisk commented Feb 22, 2016

Could you supply a piece of code to reproduce the bug?
Is the bug present in previous versions?

@ddur
Copy link
Author

ddur commented Feb 23, 2016

Test
Implementation under test
Bug was not present in 2.4.5828.26833

@ddur
Copy link
Author

ddur commented Feb 23, 2016

@ondfisk
Just add two or three Key-Value-Pair (T, int) with different int values.
Get the .Values, everything is OK.
Get the .Values again and all int values are 0.

@Dana-Ferguson
Copy link

I have found the change that caused the issue:

inside: internal class KeysCollection : CollectionValueBase<K>, ICollectionValue<K>
@ Dictionaries.cs

            public override SCG.IEnumerator<V> GetEnumerator()
            {
                //Updatecheck is performed by the pairs enumerator
                foreach (KeyValuePair<K, V> p in pairs)
                    yield return p.Value;
            }

became

            public override SCG.IEnumerator<V> GetEnumerator()
            {
                //Updatecheck is performed by the pairs enumerator
                _internalEnumerator.UpdateReference(_pairs);
                return _internalEnumerator;
            }

The new enumerator:

                internal void UpdateReference(ICollection<KeyValuePair<K, V>> list)
                {
                    _internalList = list;
                    Current = default(V);
                }

                public override bool MoveNext()
                {
                    ICollection<KeyValuePair<K, V>> list = _internalList;

                    if (_internalEnumerator == null)
                        _internalEnumerator = list.GetEnumerator();

                    if (_internalEnumerator.MoveNext())
                    {
                        Current = _internalEnumerator.Current.Value;
                        return true;
                    }

                    Current = default(V);
                    return false;
                }

And within that: (in MoveNext)

                    if (_internalEnumerator == null)
                        _internalEnumerator = list.GetEnumerator();

Is only ever called once and never updated.

if you add _internalEnumerator = list.GetEnumerator(); to UpdateReference, it works again.
I assume that its supposed to be linqy and update automagically, but its not (why this is a comment, and not a PR, I don't understand the intended behavior).

I noticed that the Keys collection resolves differently at runtime. Values uses the ValuesCollection and ValueEnumerator resolving to C5.TreeSet<C5.KeyValuePair<,>.Enumerator (which has the old stamp value of '0'), but Keys resolves to C5.SortedDictionaryBase<, >.SortedKeysCollection.KeyEnumerator. So I assume this bug would also affect Keys if Keys used the KeysCollection in the same way as the Values uses ValuesCollection.

I hope this helps in solving the problem.

@ondfisk
Copy link
Collaborator

ondfisk commented Feb 25, 2016

It certainly does. Thanks.
I will look into it ASAP.
I have hidden the buggy NuGet packages.

codingadventures pushed a commit to codingadventures/C5 that referenced this issue Feb 28, 2016
- in the ValueCollection and KeyCollection an enumerator was not
properly generated. Now it's been introduced in the GetEnumerator
function of both classes.
- to avoid confusions some fields have been renamed: _internalList to
_keyValuePairs, _internalEnumerator to _keyEnumerator and to
_valueEnumerator and so on
- another bug is fixed -->  _keyValuePairEnumerator.Dispose(); after the
collection ends looping so it can restart once it has finished
@ddur
Copy link
Author

ddur commented Feb 28, 2016

Thanks

@ddur ddur closed this as completed Feb 28, 2016
@ondfisk
Copy link
Collaborator

ondfisk commented Apr 13, 2016

I've pushed a new version to NuGet. Please verify that the issue has been resolved

@JnxF JnxF added the bug Something isn't working label Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants