Skip to content

Commit 1e20113

Browse files
NoggogNoggog
committed
Reverted some dict ext optimizations for a bugged edge case
If the func based GetOrAdds self referenced the dictionary, the results got weird, as the optimization prematurely added the key to the dict before the value was setS
1 parent e7eecd3 commit 1e20113

File tree

2 files changed

+64
-73
lines changed

2 files changed

+64
-73
lines changed

‎CSharpExt.UnitTests/DictionaryExtTests.cs‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ public void GetOrAddFunc(
1717
var s = d.GetOrAdd(key, (s) =>
1818
{
1919
s.ShouldBe(key);
20+
21+
// Edge case test
22+
if (d.TryGetValue(key, out var existing)) return existing;
23+
2024
return value;
2125
});
2226
s.ShouldBe(value);
@@ -37,6 +41,9 @@ public void GetOrAddNoKeyFunc(
3741
var d = new Dictionary<string, int>();
3842
var s = d.GetOrAdd(key, () =>
3943
{
44+
// Edge case test
45+
if (d.TryGetValue(key, out var existing)) return existing;
46+
4047
return value;
4148
});
4249
s.ShouldBe(value);
@@ -67,6 +74,10 @@ public void GetOrAddFuncIDictionary(
6774
var s = d.GetOrAdd(key, (s) =>
6875
{
6976
s.ShouldBe(key);
77+
78+
// Edge case test
79+
if (d.TryGetValue(key, out var existing)) return existing;
80+
7081
return value;
7182
});
7283
s.ShouldBe(value);
@@ -87,6 +98,9 @@ public void GetOrAddNoKeyFuncIDictionary(
8798
IDictionary<string, int> d = new Dictionary<string, int>();
8899
var s = d.GetOrAdd(key, () =>
89100
{
101+
// Edge case test
102+
if (d.TryGetValue(key, out var existing)) return existing;
103+
90104
return value;
91105
});
92106
s.ShouldBe(value);
@@ -117,6 +131,9 @@ public void GetOrAddNoKeyFuncConcurrent(
117131
var d = new ConcurrentDictionary<string, int>();
118132
var s = d.GetOrAdd(key, () =>
119133
{
134+
// Edge case test
135+
if (d.TryGetValue(key, out var existing)) return existing;
136+
120137
return value;
121138
});
122139
s.ShouldBe(value);
@@ -148,6 +165,10 @@ public void ConcurrentDictionaryGetOrAdd(
148165
var s = d.GetOrAdd(key, (s) =>
149166
{
150167
s.ShouldBe(key);
168+
169+
// Edge case test
170+
if (d.TryGetValue(key, out var existing)) return existing;
171+
151172
return value;
152173
});
153174
s.ShouldBe(value);
@@ -170,6 +191,10 @@ public void UpdateOrAddWithKey(
170191
{
171192
k.ShouldBe(key);
172193
s.ShouldBe(default(int));
194+
195+
// Edge case test
196+
if (d.TryGetValue(key, out var existing)) return existing;
197+
173198
return value;
174199
});
175200
s.ShouldBe(value);
@@ -193,6 +218,10 @@ public void IDictionaryUpdateOrAddWithKey(
193218
{
194219
k.ShouldBe(key);
195220
s.ShouldBe(default(int));
221+
222+
// Edge case test
223+
if (d.TryGetValue(key, out var existing)) return existing;
224+
196225
return value;
197226
});
198227
s.ShouldBe(value);
@@ -215,12 +244,17 @@ public void UpdateOrAdd(
215244
var s = d.UpdateOrAdd(key, (s) =>
216245
{
217246
s.ShouldBe(default(int));
247+
248+
// Edge case test
249+
if (d.TryGetValue(key, out var existing)) return existing;
250+
218251
return value;
219252
});
220253
s.ShouldBe(value);
221254
var s2 = d.UpdateOrAdd(key, (s) =>
222255
{
223256
s.ShouldBe(value);
257+
224258
return value2;
225259
});
226260
s2.ShouldBe(value2);
@@ -235,13 +269,18 @@ public void IDictionaryUpdateOrAdd(
235269
IDictionary<string, int> d = new Dictionary<string, int>();
236270
var s = d.UpdateOrAdd(key, (s) =>
237271
{
238-
s.ShouldBe(default(int));
272+
s.ShouldBe(default(int));
273+
274+
// Edge case test
275+
if (d.TryGetValue(key, out var existing)) return existing;
276+
239277
return value;
240278
});
241279
s.ShouldBe(value);
242280
var s2 = d.UpdateOrAdd(key, (s) =>
243281
{
244282
s.ShouldBe(value);
283+
245284
return value2;
246285
});
247286
s2.ShouldBe(value2);

‎Noggog.CSharpExt/Extensions/DictionaryExt.cs‎

Lines changed: 24 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -80,28 +80,6 @@ public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict,
8080
}
8181
return ret;
8282
}
83-
84-
public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key, Func<TValue> getNew)
85-
where TKey : notnull
86-
{
87-
if (!dict.TryGetValue(key, out var ret))
88-
{
89-
ret = getNew();
90-
dict[key] = ret;
91-
}
92-
return ret;
93-
}
94-
95-
public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key, Func<TKey, TValue> getNew)
96-
where TKey : notnull
97-
{
98-
if (!dict.TryGetValue(key, out var ret))
99-
{
100-
ret = getNew(key);
101-
dict[key] = ret;
102-
}
103-
return ret;
104-
}
10583
#else
10684
public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key)
10785
where TKey : notnull
@@ -117,35 +95,29 @@ public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict,
11795
val = newVal;
11896
return newVal;
11997
}
98+
#endif
12099

121100
public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key, Func<TValue> getNew)
122101
where TKey : notnull
123102
{
124-
ref var val = ref CollectionsMarshal.GetValueRefOrAddDefault(dict, key, out var exists);
125-
if (exists)
126-
{
127-
return val!;
128-
}
129-
130-
var newVal = getNew();
131-
val = newVal;
132-
return newVal;
103+
if (!dict.TryGetValue(key, out var ret))
104+
{
105+
ret = getNew();
106+
dict[key] = ret;
107+
}
108+
return ret;
133109
}
134110

135111
public static TValue GetOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key, Func<TKey, TValue> getNew)
136112
where TKey : notnull
137113
{
138-
ref var val = ref CollectionsMarshal.GetValueRefOrAddDefault(dict, key, out var exists);
139-
if (exists)
140-
{
141-
return val!;
142-
}
143-
144-
var newVal = getNew(key);
145-
val = newVal;
146-
return newVal;
114+
if (!dict.TryGetValue(key, out var ret))
115+
{
116+
ret = getNew(key);
117+
dict[key] = ret;
118+
}
119+
return ret;
147120
}
148-
#endif
149121

150122
public static TValue UpdateOrAdd<TKey, TValue>(this IDictionary<TKey, TValue> dict, TKey key, Func<TValue?, TValue> getNew)
151123
{
@@ -182,37 +154,17 @@ public static TValue UpdateOrAdd<TKey, TValue>(this IDictionary<TKey, TValue> di
182154
public static TValue UpdateOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key, Func<TValue?, TValue> getNew)
183155
where TKey : notnull
184156
{
185-
ref var val = ref CollectionsMarshal.GetValueRefOrAddDefault(dict, key, out var exists);
186-
if (exists)
187-
{
188-
var update = getNew(val);
189-
val = update;
190-
return update;
191-
}
192-
else
193-
{
194-
var newVal = getNew(default);
195-
val = newVal;
196-
return newVal;
197-
}
198-
}
199-
200-
public static TValue UpdateOrAdd<TKey, TValue>(this Dictionary<TKey, TValue> dict, TKey key, Func<TKey, TValue?, TValue> getNew)
201-
where TKey : notnull
202-
{
203-
ref var val = ref CollectionsMarshal.GetValueRefOrAddDefault(dict, key, out var exists);
204-
if (exists)
205-
{
206-
var update = getNew(key, val);
207-
val = update;
208-
return update;
209-
}
210-
else
211-
{
212-
var newVal = getNew(key, default);
213-
val = newVal;
214-
return newVal;
215-
}
157+
if (dict.TryGetValue(key, out var ret))
158+
{
159+
ret = getNew(ret);
160+
dict[key] = ret;
161+
}
162+
else
163+
{
164+
ret = getNew(default);
165+
dict[key] = ret;
166+
}
167+
return ret;
216168
}
217169
#endif
218170

0 commit comments

Comments
 (0)