Dictionary default value: retarget net10.0, fix the mis-wired tests, add the fallback overload - #2174
Open
vladimir-pecanac-main wants to merge 1 commit into
Conversation
… dynamic - All three projects retargeted net7.0 -> net10.0. - Tests: four of six tests were named for MethodTryGetValue and MethodContainsKey but all six called MethodGetValueOrDefault, so two of the three methods had zero coverage while the suite reported 6/6 green. Each test now calls the method its name claims, and the reversed Assert.AreEqual(actual, expected) arguments are swapped to AreEqual(expected, actual) so a failure message tells the truth. - Methods: dynamic / dynamic? return types replaced with T?; MethodGetValueOrDefault takes string instead of string? (clears CS8620 and CS8603 on net10.0). - New MethodGetValueOrDefaultWithFallback covering the two-argument GetValueOrDefault(key, fallback) overload, with tests proving the fallback is returned for a missing key and that a stored value equal to the fallback is indistinguishable from a missing one. - New test for FirstOrDefault on a dictionary: the default KeyValuePair is a struct, not null. - Packages: BenchmarkDotNet 0.15.8; Microsoft.NET.Test.Sdk 18.9.0, MSTest 4.3.3, coverlet.collector 10.0.1. Removed the unused BenchmarkDotNet reference and the System.Collections 4.3.0 compatibility shim from the app project.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates
collections-dictionary/DefaultValueFromDictionaryInCSharpfor the article rewrite of How to Return a Default Value From a Dictionary in C#.What changed
Retarget all three projects
net7.0->net10.0.The test suite was broken and green at the same time. Four of the six tests in
DefaultValueFromDictionaryUnitTests.cswere namedGivenMethodTryGetValue_*andGivenMethodContainsKey_*, but the file's onlyusingwasDefaultValueFromDictionaryInCSharp.MethodGetValueOrDefaultand all six tests calledMethodGetValueOrDefault.GetValueFromDictionary.MethodContainsKeyandMethodTryGetValuehad zero coverage while the suite reported 6/6 passing. Each test now calls the method its name claims, with the two missingusingdirectives added.Reversed assert arguments. Every assert was
Assert.AreEqual(value, 0). MSTest's signature isAreEqual(expected, actual), so the assertions passed either way and the failure message named the wrong side. All swapped.dynamicremoved. All three methods returneddynamic(ordynamic?) from a<T>-generic method overDictionary<string, T>, which erases the type the caller already has and adds a DLR call site. They now returnT?.MethodGetValueOrDefaulttookstring? keywhile its two siblings tookstring. That nullable parameter dragged the inferredTKeytostring?, which on net10.0 produced, atMethods/MethodGetValueOrDefault.cs(7,20):MethodContainsKey.cs(7,20)emitted CS8603 as well. Both are gone: the build is now 0 warnings, 0 errors.New
Methods/MethodGetValueOrDefaultWithFallback.cscovering the two-argumentGetValueOrDefault(key, fallback)overload, which the sample did not demonstrate at all. Its tests prove the fallback comes back for a missing key and that a stored value equal to the fallback is returned identically, so the two cases are genuinely indistinguishable.New
FirstOrDefaulttest.Dictionary<TKey, TValue>is anIEnumerable<KeyValuePair<TKey, TValue>>, soFirstOrDefault()compiles on it and returnsdefault(KeyValuePair<TKey, TValue>)for no match. That default is a struct, notnull.Packages.
BenchmarkDotNet0.13.4 -> 0.15.8.Microsoft.NET.Test.Sdk17.3.2 -> 18.9.0,MSTest.TestAdapter/MSTest.TestFramework2.2.10 -> 4.3.3,coverlet.collector3.1.2 -> 10.0.1. Removed the unusedBenchmarkDotNetreference and theSystem.Collections4.3.0 .NET Framework compatibility shim from the app project (the build itself flagged the latter:NU1510: PackageReference System.Collections will not be pruned ... likely unnecessary).Security advisory cleared, deliberately not by accident. The old
MSTest2.2.10 chain pulledNewtonsoft.Json10.0.3 transitively, which carries the high-severity advisoryNU1903(GHSA-5crp-9r3c-p9vr). The package bumps above remove it:dotnet list package --vulnerable --include-transitivenow reports no vulnerable packages in any of the three projects.Evidence
Build and tests, SDK 10.0.302:
The mis-wiring fix is proved by a deliberate break, not by a green run (a green run is what the bug already produced). Breaking
MethodContainsKeyandMethodTryGetValueon this branch fails exactly the four tests named for them; onmainthe same break failed nothing, because no test called either method.Benchmarks, re-run on net10.0
The article's published figures were measured in 2023 on
net7.0with BenchmarkDotNet 0.13.4. Both runs below are on this branch, verbatim.Run 1, key present (
_key = "number_1000"):Run 2, key absent (same binaries,
_keyedited to"number_-1"for the run and reverted):The ordering the 2023 run reported survives, and the reason is now visible in the numbers:
TryGetValue()andGetValueOrDefault()land inside each other's error bars in both runs (BenchmarkDotNet ranks them equal first), becauseGetValueOrDefault()is a wrapper overTryGetValue(). TheContainsKey()check pays for its second lookup, and it pays most when the key is present and the second lookup actually has to find something.