sestoft / C5

C5 generic collection library for C#/.NET
http://www.itu.dk/research/c5/
MIT License
1.03k stars 181 forks source link

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

Closed ddur closed 8 years ago

ddur commented 8 years ago

C5 2.4.5891.40110 (NuGet) Net 4.5.2

ddur commented 8 years ago

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

ondfisk commented 8 years ago

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

ddur commented 8 years ago

Test Implementation under test Bug was not present in 2.4.5828.26833

ddur commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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

ddur commented 8 years ago

Thanks

ondfisk commented 8 years ago

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