V10.2.0/support for m49 - #142
Conversation
📝 WalkthroughWalkthroughAdds UN M.49 statistical region support: new domain types and enum, a CSV-backed lazy data container, World lookup APIs, extension predicates, embedded resource registration, tests, release notes and docs/metadata updates (years, prompts, changelog, license). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TestRunner
participant World
participant UnM49DataContainer
participant EmbeddedResource as "unm49-data.csv (embedded)"
participant StatisticalRegion as "StatisticalRegionInfo"
TestRunner->>World: Call GetCountry/GetStatisticalRegion/Enumerate StatisticalRegions
World->>UnM49DataContainer: Ensure initialized (Lazy)
UnM49DataContainer->>EmbeddedResource: Read CSV stream
UnM49DataContainer->>UnM49DataContainer: Parse lines -> create region/country objects
UnM49DataContainer->>StatisticalRegion: Instantiate StatisticalRegionInfo nodes
UnM49DataContainer->>UnM49DataContainer: Build parent/child relationships & validations
UnM49DataContainer-->>World: Expose RegionsByCode, CountriesByCode, CountriesByIsoAlpha2
World-->>TestRunner: Return requested StatisticalRegionInfo / collections
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/Cuemon.Core/Cuemon.Core.csproj`:
- Line 13: The PackageTags entry contains a typo: the tag "countr" should be
corrected to "country"; update the <PackageTags> element in the project file so
the tag list includes "country" instead of "countr" (ensure you only change that
token and keep the rest of the tags intact).
In `@src/Cuemon.Core/Globalization/unm49-data.json`:
- Around line 2490-2588: Several country entries are incorrectly marked as lldc:
true; update the lldc flag to false for the developed countries listed (e.g.,
"code":"040" Austria, "438" Liechtenstein, "442" Luxembourg, "756" Switzerland)
and also set lldc to false for Georgia ("268") and the other developed
microstates referenced (Andorra, Holy See, San Marino) so that only the official
UN OHRLLS LLDCs remain true; locate each country object in unm49-data.json by
its "code" or "name" and change the "lldc" property from true to false.
In `@test/Cuemon.Core.Tests/Globalization/StatisticalRegionInfoTest.cs`:
- Around line 186-194: The test method
GetAllCountries_ShouldReturn250PlusCountries currently asserts
world.Countries.Count >= 200 which mismatches the method name; update the
assertion in that test (referencing world = World.GetStatisticalRegion("001")
and world.Countries.Count) to assert >= 250 to match the method name, or
alternatively rename the test to GetAllCountries_ShouldReturn200PlusCountries if
you intend to keep the lower threshold.
- Around line 248-262: The test IsoCodes_ShouldBeUppercase currently only checks
that country.IsoAlpha2 and country.IsoAlpha3 contain letters and have correct
lengths; update this test to also assert the codes are uppercase by adding
checks (for example using All(char.IsUpper) or comparing to ToUpperInvariant())
for both country.IsoAlpha2 and country.IsoAlpha3 inside the loop that iterates
World.GetStatisticalRegion("001").Countries so the test name matches its
behavior.
In
`@test/Cuemon.Extensions.Core.Tests/Globalization/StatisticalRegionExtensionsTest.cs`:
- Line 10: The test class is misnamed—rename the class
StatisticalRegionExtensions to StatisticalRegionExtensionsTest; update the class
declaration (class StatisticalRegionExtensions -> class
StatisticalRegionExtensionsTest) and any references to it (e.g., test fixtures
or usages) so the test class name follows the "Test" suffix convention and
compiles correctly with the test runner.
🧹 Nitpick comments (4)
src/Cuemon.Core/Globalization/UnM49DataContainer.cs (2)
31-47: Unreachable null check —Single()throws before stream can be null.
GetManifestResources(resourceName).Single()will throwInvalidOperationExceptionif no element is found (or more than one), so thestream == nullcheck on Line 34 is never reached. Consider removing the dead branch or replacingSingle()withSingleOrDefault()if you want the null-check path to be reachable.
86-95: Barecatchswallows all exceptions.The
catchon Line 92 catches every exception type, including catastrophic ones (e.g.,OutOfMemoryException). Since the intent is only to handle unsupported territories, narrow the catch to the expected exception type(s).Proposed fix
try { regionInfo = World.Regions.FirstOrDefault(r => string.Equals(r.TwoLetterISORegionName, countryData.IsoAlpha2, StringComparison.OrdinalIgnoreCase)); } - catch + catch (Exception) when (regionInfo == null) { // Some territories may not be supported by the OS }Or more precisely, catch only the expected
ArgumentException/CultureNotFoundExceptiontypes.src/Cuemon.Core/Globalization/World.cs (1)
35-51: Eager evaluation ofLazywrappers defeats deferred loading.Both
Regions(Line 41) andStatisticalRegions(Line 51) are auto-property initializers that unwrapLazy.Valueduring static construction, so the JSON resource is parsed the first time anyWorldmember is accessed — even if the caller only needsGetCultures. TheLazy<T>wrappers serve no deferral purpose here since the static constructor eagerly evaluates all field initializers in order.This is fine if the cost is acceptable at app startup. If you'd prefer to defer the M49 load until actually needed, make
StatisticalRegionsdelegate to theLazyrather than capture its value:Example
-public static IReadOnlyList<StatisticalRegionInfo> StatisticalRegions { get; } = UnM49Data.Value.Regions; +public static IReadOnlyList<StatisticalRegionInfo> StatisticalRegions => UnM49Data.Value.Regions;(Same approach could be applied to
Regionsif desired.)src/Cuemon.Core/Globalization/StatisticalRegionInfo.cs (1)
120-122:Countriesallocates a new list on every access.
Countriestraverses the full descendant tree and materialises a newList<T>each time it is read. For the World node this means ~250+ items re-collected on every call, which is hidden behind a property that callers will assume is cheap. Consider caching the result (e.g., aLazy<IReadOnlyList<…>>) or at minimum switching to a method name (GetCountries()) to signal the cost.Caching example
+private IReadOnlyList<StatisticalRegionInfo> _countries; + public IReadOnlyList<StatisticalRegionInfo> Countries => - GetAllDescendants() - .Where(r => r.Kind == StatisticalRegionKind.CountryOrTerritory) - .ToList(); + _countries ??= GetAllDescendants() + .Where(r => r.Kind == StatisticalRegionKind.CountryOrTerritory) + .ToList();
| { | ||
| "code": "040", | ||
| "name": "Austria", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "AT", | ||
| "isoAlpha3": "AUT", | ||
| "ldc": false, | ||
| "lldc": true, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "056", | ||
| "name": "Belgium", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "BE", | ||
| "isoAlpha3": "BEL", | ||
| "ldc": false, | ||
| "lldc": false, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "250", | ||
| "name": "France", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "FR", | ||
| "isoAlpha3": "FRA", | ||
| "ldc": false, | ||
| "lldc": false, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "276", | ||
| "name": "Germany", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "DE", | ||
| "isoAlpha3": "DEU", | ||
| "ldc": false, | ||
| "lldc": false, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "438", | ||
| "name": "Liechtenstein", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "LI", | ||
| "isoAlpha3": "LIE", | ||
| "ldc": false, | ||
| "lldc": true, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "442", | ||
| "name": "Luxembourg", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "LU", | ||
| "isoAlpha3": "LUX", | ||
| "ldc": false, | ||
| "lldc": true, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "492", | ||
| "name": "Monaco", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "MC", | ||
| "isoAlpha3": "MCO", | ||
| "ldc": false, | ||
| "lldc": false, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "528", | ||
| "name": "Netherlands (Kingdom of the)", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "NL", | ||
| "isoAlpha3": "NLD", | ||
| "ldc": false, | ||
| "lldc": false, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, | ||
| { | ||
| "code": "756", | ||
| "name": "Switzerland", | ||
| "parentCode": "155", | ||
| "isoAlpha2": "CH", | ||
| "isoAlpha3": "CHE", | ||
| "ldc": false, | ||
| "lldc": true, | ||
| "sids": false, | ||
| "kind": "CountryOrTerritory" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Extract all entries with lldc: true from the JSON file
jq '.countries[] | select(.lldc == true) | {code, name, lldc}' src/Cuemon.Core/Globalization/unm49-data.json | head -50Repository: codebeltnet/cuemon
Length of output: 647
🌐 Web query:
UN OHRLLS Landlocked Developing Countries official list 2024 2025
💡 Result:
UN-OHRLLS’ official list of Landlocked Developing Countries (LLDCs) (used in 2024 and 2025; 32 countries) is: Afghanistan, Armenia, Azerbaijan, Bhutan, Botswana, Burkina Faso, Burundi, Central African Republic, Chad, Eswatini, Ethiopia, Kazakhstan, Kyrgyzstan, Lao People’s Democratic Republic, Lesotho, Malawi, Mali, Mongolia, Nepal, Niger, North Macedonia, Paraguay, Plurinational State of Bolivia, Republic of Moldova, Rwanda, South Sudan, Tajikistan, Turkmenistan, Uganda, Uzbekistan, Zambia, Zimbabwe. [1]
This matches the UN’s general LLDC explainer page and UNCTAD’s LLDC list (also 32). [2], [3]
Sources: UN-OHRLLS “List of LLDCs” [1]; UN “About Landlocked Developing Countries” [2]; UNCTAD “List of Landlocked Developing Countries” [3].
🏁 Script executed:
# Count total entries with lldc: true
jq '.countries[] | select(.lldc == true)' src/Cuemon.Core/Globalization/unm49-data.json | jq -s 'length'
# Check specific countries mentioned in review comment
echo "=== Checking developed countries mentioned in review ==="
for code in 040 756 442 438 020 336 674 203 348; do
jq ".countries[] | select(.code == \"$code\") | {code, name, lldc}" src/Cuemon.Core/Globalization/unm49-data.json
done
# Get complete list of all countries with lldc: true
echo "=== Complete list of countries with lldc: true ==="
jq '.countries[] | select(.lldc == true) | {code, name}' src/Cuemon.Core/Globalization/unm49-data.jsonRepository: codebeltnet/cuemon
Length of output: 2472
Correct the lldc field: 8 developed countries incorrectly marked as Landlocked Developing Countries.
The JSON file marks Austria, Liechtenstein, Luxembourg, Switzerland, Andorra, Holy See, and San Marino as lldc: true. The UN OHRLLS official LLDC list (32 countries, 2024–2025) designates LLDCs exclusively as developing countries that lack sea access—not developed ones. Austria, Switzerland, Luxembourg, Liechtenstein, Andorra, Holy See, and San Marino must be set to "lldc": false.
Additionally, Georgia (268) is marked lldc: true but is not on the UN OHRLLS LLDC list and should also be corrected to false.
All 32 official UN LLDCs are otherwise correctly included in the data.
🤖 Prompt for AI Agents
In `@src/Cuemon.Core/Globalization/unm49-data.json` around lines 2490 - 2588,
Several country entries are incorrectly marked as lldc: true; update the lldc
flag to false for the developed countries listed (e.g., "code":"040" Austria,
"438" Liechtenstein, "442" Luxembourg, "756" Switzerland) and also set lldc to
false for Georgia ("268") and the other developed microstates referenced
(Andorra, Holy See, San Marino) so that only the official UN OHRLLS LLDCs remain
true; locate each country object in unm49-data.json by its "code" or "name" and
change the "lldc" property from true to false.
| public void GetAllCountries_ShouldReturn250PlusCountries() | ||
| { | ||
| var world = World.GetStatisticalRegion("001"); | ||
|
|
||
| Assert.NotNull(world); | ||
| Assert.True(world.Countries.Count >= 200, $"Expected at least 200 countries, got {world.Countries.Count}"); | ||
|
|
||
| TestOutput.WriteLine($"Total countries: {world.Countries.Count}"); | ||
| } |
There was a problem hiding this comment.
Test method name says "250Plus" but assertion checks ≥ 200.
The method is named GetAllCountries_ShouldReturn250PlusCountries but the assertion on Line 191 checks >= 200. Either tighten the assertion to match the name or rename the method.
🤖 Prompt for AI Agents
In `@test/Cuemon.Core.Tests/Globalization/StatisticalRegionInfoTest.cs` around
lines 186 - 194, The test method GetAllCountries_ShouldReturn250PlusCountries
currently asserts world.Countries.Count >= 200 which mismatches the method name;
update the assertion in that test (referencing world =
World.GetStatisticalRegion("001") and world.Countries.Count) to assert >= 250 to
match the method name, or alternatively rename the test to
GetAllCountries_ShouldReturn200PlusCountries if you intend to keep the lower
threshold.
| [Fact] | ||
| public void IsoCodes_ShouldBeUppercase() | ||
| { | ||
| var world = World.GetStatisticalRegion("001"); | ||
|
|
||
| foreach (var country in world.Countries) | ||
| { | ||
| Assert.True(country.IsoAlpha2.All(char.IsLetter), | ||
| $"Country {country.Name} should have valid ISO Alpha-2 code"); | ||
| Assert.True(country.IsoAlpha3.All(char.IsLetter), | ||
| $"Country {country.Name} should have valid ISO Alpha-3 code"); | ||
| Assert.Equal(2, country.IsoAlpha2.Length); | ||
| Assert.Equal(3, country.IsoAlpha3.Length); | ||
| } | ||
| } |
There was a problem hiding this comment.
Test verifies letters but not uppercase — name is misleading.
IsoCodes_ShouldBeUppercase only asserts char.IsLetter and correct length, but never checks that the codes are actually uppercase. Add an uppercase assertion to match the test name.
Proposed addition
foreach (var country in world.Countries)
{
+ Assert.Equal(country.IsoAlpha2, country.IsoAlpha2.ToUpperInvariant());
+ Assert.Equal(country.IsoAlpha3, country.IsoAlpha3.ToUpperInvariant());
Assert.True(country.IsoAlpha2.All(char.IsLetter),
$"Country {country.Name} should have valid ISO Alpha-2 code");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [Fact] | |
| public void IsoCodes_ShouldBeUppercase() | |
| { | |
| var world = World.GetStatisticalRegion("001"); | |
| foreach (var country in world.Countries) | |
| { | |
| Assert.True(country.IsoAlpha2.All(char.IsLetter), | |
| $"Country {country.Name} should have valid ISO Alpha-2 code"); | |
| Assert.True(country.IsoAlpha3.All(char.IsLetter), | |
| $"Country {country.Name} should have valid ISO Alpha-3 code"); | |
| Assert.Equal(2, country.IsoAlpha2.Length); | |
| Assert.Equal(3, country.IsoAlpha3.Length); | |
| } | |
| } | |
| [Fact] | |
| public void IsoCodes_ShouldBeUppercase() | |
| { | |
| var world = World.GetStatisticalRegion("001"); | |
| foreach (var country in world.Countries) | |
| { | |
| Assert.Equal(country.IsoAlpha2, country.IsoAlpha2.ToUpperInvariant()); | |
| Assert.Equal(country.IsoAlpha3, country.IsoAlpha3.ToUpperInvariant()); | |
| Assert.True(country.IsoAlpha2.All(char.IsLetter), | |
| $"Country {country.Name} should have valid ISO Alpha-2 code"); | |
| Assert.True(country.IsoAlpha3.All(char.IsLetter), | |
| $"Country {country.Name} should have valid ISO Alpha-3 code"); | |
| Assert.Equal(2, country.IsoAlpha2.Length); | |
| Assert.Equal(3, country.IsoAlpha3.Length); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@test/Cuemon.Core.Tests/Globalization/StatisticalRegionInfoTest.cs` around
lines 248 - 262, The test IsoCodes_ShouldBeUppercase currently only checks that
country.IsoAlpha2 and country.IsoAlpha3 contain letters and have correct
lengths; update this test to also assert the codes are uppercase by adding
checks (for example using All(char.IsUpper) or comparing to ToUpperInvariant())
for both country.IsoAlpha2 and country.IsoAlpha3 inside the loop that iterates
World.GetStatisticalRegion("001").Countries so the test name matches its
behavior.
|
|
||
| namespace Cuemon.Extensions.Globalization | ||
| { | ||
| public class StatisticalRegionExtensions : Test |
There was a problem hiding this comment.
Test class must end with Test.
The class is named StatisticalRegionExtensions but per coding guidelines it should be StatisticalRegionExtensionsTest.
Fix
- public class StatisticalRegionExtensions : Test
+ public class StatisticalRegionExtensionsTest : TestAs per coding guidelines, "Test classes must end with Test (e.g., DateSpanTest)".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public class StatisticalRegionExtensions : Test | |
| public class StatisticalRegionExtensionsTest : Test |
🤖 Prompt for AI Agents
In
`@test/Cuemon.Extensions.Core.Tests/Globalization/StatisticalRegionExtensionsTest.cs`
at line 10, The test class is misnamed—rename the class
StatisticalRegionExtensions to StatisticalRegionExtensionsTest; update the class
declaration (class StatisticalRegionExtensions -> class
StatisticalRegionExtensionsTest) and any references to it (e.g., test fixtures
or usages) so the test class name follows the "Test" suffix convention and
compiles correctly with the test runner.
There was a problem hiding this comment.
Pull request overview
Adds UN M.49 statistical region + country metadata support to Cuemon.Core (with lookups via World), plus companion extension methods and tests; also updates repo/package metadata and documentation.
Changes:
- Introduces
StatisticalRegionInfo/StatisticalRegionKindand loads UN M.49 hierarchy from an embeddedunm49-data.json. - Extends
Worldwith M.49 accessors (StatisticalRegions,GetStatisticalRegion,GetCountry) and addsCuemon.Extensions.Globalizationhelpers. - Updates package tags/copyright years, and adds
AGENTS.md.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Cuemon.Extensions.Core.Tests/Globalization/StatisticalRegionExtensionsTest.cs | Adds tests for new statistical region extension methods. |
| test/Cuemon.Core.Tests/Globalization/StatisticalRegionInfoTest.cs | Adds tests validating M.49 data loading, hierarchy, and lookups. |
| src/Cuemon.Extensions.Core/Globalization/StatisticalRegionExtensions.cs | Adds extension methods for classifying regions/countries and checking ISO/RegionInfo availability. |
| src/Cuemon.Core/Globalization/unm49-data.json | Adds embedded UN M.49 regions + countries dataset used to build hierarchy. |
| src/Cuemon.Core/Globalization/World.cs | Exposes M.49 regions/countries via World and provides lookup helpers. |
| src/Cuemon.Core/Globalization/Unm49Data.cs | Adds internal DTOs for JSON deserialization. |
| src/Cuemon.Core/Globalization/UnM49DataContainer.cs | Loads embedded JSON, builds hierarchy, and maintains lookup dictionaries. |
| src/Cuemon.Core/Globalization/StatisticalRegionKind.cs | Adds enum describing M.49 hierarchy levels. |
| src/Cuemon.Core/Globalization/StatisticalRegionInfo.cs | Adds core model object for regions/countries, traversal, and metadata. |
| src/Cuemon.Core/Cuemon.Core.csproj | Embeds the JSON dataset, adds tags, and references System.Text.Json for netstandard2.0. |
| LICENSE.md | Updates copyright year. |
| AGENTS.md | Adds contributor guidance for agentic coding tools and repo conventions. |
| .github/prompts/nuget.prompt.md | Minor formatting adjustment to version display. |
| .docfx/docfx.json | Updates footer copyright year. |
| /// <summary> | ||
| /// Gets the direct child regions or countries of this region. | ||
| /// </summary> | ||
| /// <value>A read-only list of children. Empty list if this is a leaf node (country).</value> | ||
| public IReadOnlyList<StatisticalRegionInfo> Children => _children; |
There was a problem hiding this comment.
Children is documented as a read-only list, but it exposes the backing List<T> instance via IReadOnlyList<T> (callers can downcast and mutate it). To keep the hierarchy invariant, return a truly read-only wrapper (e.g., AsReadOnly()/ReadOnlyCollection) or store children in an immutable collection.
| namespace Cuemon.Extensions.Globalization | ||
| { | ||
| public class StatisticalRegionExtensions : Test | ||
| { | ||
| public StatisticalRegionExtensions(ITestOutputHelper output) : base(output) |
There was a problem hiding this comment.
Test class name does not follow the repo convention of ending with Test and it also collides (same namespace + type name) with the production Cuemon.Extensions.Globalization.StatisticalRegionExtensions extension class, which can make type resolution confusing. Rename the test class to StatisticalRegionExtensionsTest (namespace can stay aligned with the SUT).
| var world = World.StatisticalRegions.FirstOrDefault(r => r.Code == "001"); | ||
| Assert.True(world.IsWorld()); | ||
| } |
There was a problem hiding this comment.
FirstOrDefault can return null here, which would produce a NullReferenceException when calling world.IsWorld() and hide the real failure reason. Add an explicit Assert.NotNull(world) (or use Single/First with a clear assertion) before invoking extension methods.
| /// The list includes the World region (code "001") and all geographic regions. | ||
| /// The collection is immutable and cached for the lifetime of the application. | ||
| /// </remarks> | ||
| public static IReadOnlyList<StatisticalRegionInfo> StatisticalRegions { get; } = UnM49Data.Value.Regions; | ||
|
|
There was a problem hiding this comment.
The XML docs say the statistical regions collection is immutable, but this returns the underlying List<StatisticalRegionInfo> from the container. Callers can downcast and mutate it. Expose a truly read-only view (e.g., ReadOnlyCollection/ImmutableArray) and consider avoiding UnM49Data.Value in the static initializer so UN M.49 data is loaded only when needed.
| [Fact] | ||
| public void IsoCodes_ShouldBeUppercase() | ||
| { | ||
| var world = World.GetStatisticalRegion("001"); | ||
|
|
There was a problem hiding this comment.
The test name says ISO codes should be uppercase, but the assertions in this test only check that the codes contain letters and have the expected length. This won’t fail for lowercase values. Add an assertion that verifies the codes are already uppercase (e.g., compare to ToUpperInvariant()).
| foreach (var countryData in data.Countries) | ||
| { | ||
| if (RegionsByCode.TryGetValue(countryData.ParentCode, out var parent)) | ||
| { | ||
| // Validate kind is CountryOrTerritory | ||
| var kind = ParseKind(countryData.Kind, countryData.Code, countryData.Name); | ||
| if (kind != StatisticalRegionKind.CountryOrTerritory) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Country {countryData.Name} ({countryData.Code}) must have kind 'CountryOrTerritory', but was '{countryData.Kind}'."); | ||
| } | ||
|
|
||
| // Try to find matching RegionInfo | ||
| RegionInfo regionInfo = null; | ||
| try | ||
| { | ||
| regionInfo = World.Regions.FirstOrDefault(r => string.Equals(r.TwoLetterISORegionName, countryData.IsoAlpha2, StringComparison.OrdinalIgnoreCase)); | ||
| } | ||
| catch | ||
| { | ||
| // Some territories may not be supported by the OS | ||
| } | ||
|
|
||
| var country = new StatisticalRegionInfo( | ||
| countryData.Code, | ||
| countryData.Name, | ||
| countryData.IsoAlpha2, | ||
| countryData.IsoAlpha3, | ||
| parent, | ||
| countryData.Ldc, | ||
| countryData.Lldc, | ||
| countryData.Sids, | ||
| regionInfo); | ||
|
|
||
| CountriesByCode[countryData.Code] = country; | ||
| if (!string.IsNullOrEmpty(countryData.IsoAlpha2)) | ||
| { | ||
| CountriesByIsoAlpha2[countryData.IsoAlpha2] = country; | ||
| } | ||
|
|
||
| // Add country as child of its immediate parent region | ||
| parent.AddChild(country); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var region in Regions) | ||
| { | ||
| if (region.Code != "001" && region.Parent == null) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Region {region.Name} ({region.Code}) must have a parent."); | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
| foreach (var code in expectedContinents) | ||
| { | ||
| var continent = World.GetStatisticalRegion(code); | ||
| Assert.NotNull(continent); | ||
| Assert.Equal("001", continent.Parent?.Code); | ||
| Assert.Equal(StatisticalRegionKind.Region, continent.Kind); | ||
| TestOutput.WriteLine(continent.ToString()); | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var region in World.StatisticalRegions) | ||
| { | ||
| var depth = GetDepth(region); | ||
| if (depth > maxDepth) | ||
| { | ||
| maxDepth = depth; | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
| foreach (var code in expectedContinents) | ||
| { | ||
| var continent = World.GetStatisticalRegion(code); | ||
| Assert.True(continent.IsRegion()); | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
@.nuget/Cuemon.Extensions.AspNetCore.Mvc.Formatters.Text.Json/PackageReleaseNotes.txt:
- Line 1: The version header currently reads "Version: 10.2.0" which breaks the
file's established pattern; update that header to "Version 10.2.0" (remove the
colon) so it matches previous entries and preserves consistency for the version
header string "Version 10.2.0".
In
@.nuget/Cuemon.Extensions.AspNetCore.Mvc.Formatters.Xml/PackageReleaseNotes.txt:
- Line 1: The release notes line currently reads "Version: 10.2.0" which is
inconsistent with prior entries; update that entry to remove the colon so it
reads "Version 10.2.0" to match the existing formatting convention (search for
the string "Version: 10.2.0" and change it to "Version 10.2.0").
In @.nuget/Cuemon.Extensions.AspNetCore.Text.Json/PackageReleaseNotes.txt:
- Line 1: The version header currently reads "Version: 10.2.0" which is
inconsistent with other entries; update the header string to match the existing
format by removing the colon so it reads "Version 10.2.0" (replace the exact
text "Version: 10.2.0" in the file with "Version 10.2.0").
In @.nuget/Cuemon.Extensions.AspNetCore/PackageReleaseNotes.txt:
- Around line 1-2: The release note header uses "Version: 10.2.0" with a colon
but earlier entries use the format "Version 10.1.2" (no colon); update the
string "Version: 10.2.0" to match the prior format (remove the colon so it reads
"Version 10.2.0") to keep formatting consistent across entries.
In @.nuget/Cuemon.Extensions.Collections.Specialized/PackageReleaseNotes.txt:
- Line 1: Replace the inconsistent "Version: 10.2.0" header by removing the
colon so it matches the established format used elsewhere (e.g., change the
string "Version: 10.2.0" to "Version 10.2.0" in the PackageReleaseNotes.txt);
ensure any similar entries follow the "Version X.X.X" pattern without a colon.
In @.nuget/Cuemon.Extensions.Core/PackageReleaseNotes.txt:
- Line 8: Update the release note sentence for the StatisticalRegionExtensions
class to correct subject-verb agreement: change "class...that consist of
extension methods" to "class...that consists of extension methods" so the
singular subject "class" matches the singular verb "consists"; locate the phrase
mentioning StatisticalRegionExtensions and replace "consist" with "consists".
In @.nuget/Cuemon.Extensions.Threading/PackageReleaseNotes.txt:
- Around line 1-6: The release note line "CHANGED Dependencies have been
upgraded..." is misleading because this package contains no external
PackageReference entries; update the PackageReleaseNotes.txt to either
explicitly name the internal project dependency (e.g., "CHANGED Cuemon.Threading
ProjectReference updated to X.Y.Z" or "CHANGED Cuemon.Threading updated to
10.2.0") if you actually bumped the internal reference, or remove/replace the
generic dependency claim with a precise note stating no external NuGet
dependencies were changed; reference the Cuemon.Threading ProjectReference and
the PackageReleaseNotes.txt entry when making the edit.
In @.nuget/Cuemon.Security.Cryptography/PackageReleaseNotes.txt:
- Line 1: Standardize the release notes header by removing the colon in the
version line so it matches prior entries (change "Version: 10.2.0" to "Version
10.2.0"); update the header in PackageReleaseNotes.txt to use the same "Version
<number>" format to avoid tooling/parsing inconsistencies.
🧹 Nitpick comments (5)
.nuget/Cuemon.Data.Integrity/PackageReleaseNotes.txt (1)
1-5: Release notes for 10.2.0 look incomplete.The PR adds UN M.49 statistical region support and new public APIs, but the 10.2.0 notes only mention dependency upgrades. Please add a bullet for the UN M.49 feature so users see the change in package release notes.
.nuget/Cuemon.Extensions.Data/PackageReleaseNotes.txt (1)
1-6: LGTM! Format and content are consistent.The release notes entry follows the established pattern perfectly—version, availability, and ALM section with dependency upgrades. The minor version bump (10.1.2 → 10.2.0) appropriately signals that new capabilities are available in the ecosystem (UN M.49 support in Cuemon.Core).
📝 Optional enhancement: Consider mentioning UN M.49 support
If Cuemon.Extensions.Data exposes or benefits from the new UN M.49 statistical region features added to Cuemon.Core, you could optionally add a note under the ALM section:
# ALM - CHANGED Dependencies have been upgraded to the latest compatible versions for all supported target frameworks (TFMs) +- NOTE Dependency upgrade enables access to new UN M.49 statistical regions support (via Cuemon.Core 10.2.0)However, given that most releases in this file use the generic dependency message, the current version is perfectly acceptable.
.nuget/Cuemon.Extensions.DependencyInjection/PackageReleaseNotes.txt (1)
1-6: Consider adding ecosystem context for users.While the dependency update note is accurate for this package, users might benefit from a brief mention that this release aligns with Cuemon 10.2.0, which adds UN M.49 statistical region support in related packages (Cuemon.Core, Cuemon.Globalization). This would provide helpful context without overstating changes to this specific package.
Example addition:
# ALM - CHANGED Dependencies have been upgraded to the latest compatible versions for all supported target frameworks (TFMs) +- NOTE This release aligns with Cuemon 10.2.0, which adds UN M.49 statistical region and country support in core packages.nuget/Cuemon.Data.SqlClient/PackageReleaseNotes.txt (1)
1-5: Consider making release notes more informative.While the current format is consistent, the identical ALM note about dependency upgrades appears in 10 consecutive releases (10.1.2 through 9.0.1). For better user experience, consider either specifying which dependencies were upgraded and to which versions, or omitting the note when changes are identical to the previous release.
.nuget/Cuemon.Core/PackageReleaseNotes.txt (1)
12-12: Clarify the GetStatisticalRegions method description.The description states "retrieves a list of statistical regions for a given UN M.49 country code," which is ambiguous. Consider clarifying:
- If this method returns parent regions containing the specified country, the description should state "retrieves all parent statistical regions for a given UN M.49 country code"
- If the parameter accepts both region and country codes, consider using "UN M.49 code" instead of "country code"
| @@ -1,3 +1,9 @@ | |||
| Version: 10.2.0 | |||
There was a problem hiding this comment.
Fix formatting inconsistency in version header.
The new version entry uses Version: 10.2.0 (with a colon), while all previous entries use the format Version 10.1.2 (without a colon). This breaks the established formatting pattern in the file.
📝 Proposed fix for consistency
-Version: 10.2.0
+Version 10.2.0
Availability: .NET 10 and .NET 9📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Version: 10.2.0 | |
| Version 10.2.0 |
🤖 Prompt for AI Agents
In
@.nuget/Cuemon.Extensions.AspNetCore.Mvc.Formatters.Text.Json/PackageReleaseNotes.txt
at line 1, The version header currently reads "Version: 10.2.0" which breaks the
file's established pattern; update that header to "Version 10.2.0" (remove the
colon) so it matches previous entries and preserves consistency for the version
header string "Version 10.2.0".
| @@ -1,3 +1,9 @@ | |||
| Version: 10.2.0 | |||
There was a problem hiding this comment.
Align “Version” formatting with prior entries.
Line 1 uses Version: but prior entries use Version without a colon. Consistency matters in user-facing release notes.
🤖 Prompt for AI Agents
In
@.nuget/Cuemon.Extensions.AspNetCore.Mvc.Formatters.Xml/PackageReleaseNotes.txt
at line 1, The release notes line currently reads "Version: 10.2.0" which is
inconsistent with prior entries; update that entry to remove the colon so it
reads "Version 10.2.0" to match the existing formatting convention (search for
the string "Version: 10.2.0" and change it to "Version 10.2.0").
| @@ -1,3 +1,9 @@ | |||
| Version: 10.2.0 | |||
There was a problem hiding this comment.
Fix formatting inconsistency in version header.
The version header uses "Version: 10.2.0" with a colon, but all other version entries in this file use "Version X.Y.Z" without a colon (see lines 7, 13, 19, 25, etc.). Please remove the colon for consistency.
📝 Proposed fix
-Version: 10.2.0
+Version 10.2.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Version: 10.2.0 | |
| Version 10.2.0 |
🤖 Prompt for AI Agents
In @.nuget/Cuemon.Extensions.AspNetCore.Text.Json/PackageReleaseNotes.txt at
line 1, The version header currently reads "Version: 10.2.0" which is
inconsistent with other entries; update the header string to match the existing
format by removing the colon so it reads "Version 10.2.0" (replace the exact
text "Version: 10.2.0" in the file with "Version 10.2.0").
| Version: 10.2.0 | ||
| Availability: .NET 10 and .NET 9 |
There was a problem hiding this comment.
Align version label formatting with prior entries.
Line 1 uses Version: 10.2.0 while all earlier blocks use Version 10.1.2 (no colon). Consider keeping the same format for consistency.
Proposed fix
-Version: 10.2.0
+Version 10.2.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Version: 10.2.0 | |
| Availability: .NET 10 and .NET 9 | |
| Version 10.2.0 | |
| Availability: .NET 10 and .NET 9 |
🤖 Prompt for AI Agents
In @.nuget/Cuemon.Extensions.AspNetCore/PackageReleaseNotes.txt around lines 1 -
2, The release note header uses "Version: 10.2.0" with a colon but earlier
entries use the format "Version 10.1.2" (no colon); update the string "Version:
10.2.0" to match the prior format (remove the colon so it reads "Version
10.2.0") to keep formatting consistent across entries.
| @@ -1,3 +1,9 @@ | |||
| Version: 10.2.0 | |||
There was a problem hiding this comment.
Fix formatting inconsistency: remove colon after "Version".
The version header uses "Version: 10.2.0" with a colon, but all previous version entries (lines 7, 13, 19, 25, etc.) follow the pattern "Version X.X.X" without a colon. This breaks consistency with the established format.
📝 Proposed fix
-Version: 10.2.0
+Version 10.2.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Version: 10.2.0 | |
| Version 10.2.0 |
🤖 Prompt for AI Agents
In @.nuget/Cuemon.Extensions.Collections.Specialized/PackageReleaseNotes.txt at
line 1, Replace the inconsistent "Version: 10.2.0" header by removing the colon
so it matches the established format used elsewhere (e.g., change the string
"Version: 10.2.0" to "Version 10.2.0" in the PackageReleaseNotes.txt); ensure
any similar entries follow the "Version X.X.X" pattern without a colon.
| - CHANGED Dependencies have been upgraded to the latest compatible versions for all supported target frameworks (TFMs) | ||
|
|
||
| # New Features | ||
| - ADDED StatisticalRegionExtensions class in the Cuemon.Extensions.Globalization that consist of extension methods for the StatisticalRegionInfo class: IsWorld, IsRegion, IsSubregion, IsIntermediateRegion, IsCountryOrTerritory, IsArea, HasIsoCodes and HasRegionInfo |
There was a problem hiding this comment.
Fix subject-verb agreement.
Line 8: "class...that consist of" should be "class...that consists of" (singular subject requires singular verb form).
📝 Proposed fix
-- ADDED StatisticalRegionExtensions class in the Cuemon.Extensions.Globalization that consist of extension methods for the StatisticalRegionInfo class: IsWorld, IsRegion, IsSubregion, IsIntermediateRegion, IsCountryOrTerritory, IsArea, HasIsoCodes and HasRegionInfo
+- ADDED StatisticalRegionExtensions class in the Cuemon.Extensions.Globalization that consists of extension methods for the StatisticalRegionInfo class: IsWorld, IsRegion, IsSubregion, IsIntermediateRegion, IsCountryOrTerritory, IsArea, HasIsoCodes and HasRegionInfo📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - ADDED StatisticalRegionExtensions class in the Cuemon.Extensions.Globalization that consist of extension methods for the StatisticalRegionInfo class: IsWorld, IsRegion, IsSubregion, IsIntermediateRegion, IsCountryOrTerritory, IsArea, HasIsoCodes and HasRegionInfo | |
| - ADDED StatisticalRegionExtensions class in the Cuemon.Extensions.Globalization that consists of extension methods for the StatisticalRegionInfo class: IsWorld, IsRegion, IsSubregion, IsIntermediateRegion, IsCountryOrTerritory, IsArea, HasIsoCodes and HasRegionInfo |
🤖 Prompt for AI Agents
In @.nuget/Cuemon.Extensions.Core/PackageReleaseNotes.txt at line 8, Update the
release note sentence for the StatisticalRegionExtensions class to correct
subject-verb agreement: change "class...that consist of extension methods" to
"class...that consists of extension methods" so the singular subject "class"
matches the singular verb "consists"; locate the phrase mentioning
StatisticalRegionExtensions and replace "consist" with "consists".
| Version: 10.2.0 | ||
| Availability: .NET 10, .NET 9 and .NET Standard 2.0 | ||
|
|
||
| # ALM | ||
| - CHANGED Dependencies have been upgraded to the latest compatible versions for all supported target frameworks (TFMs) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that Cuemon.Extensions.Threading has actual dependency upgrades for version 10.2.0
# Find the project file
fd -e csproj "Cuemon.Extensions.Threading.csproj" 2>/dev/null | head -20
# Check for PackageReference entries to see what dependencies exist
echo "=== PackageReferences ==="
find . -name "Cuemon.Extensions.Threading.csproj" -exec grep -A 2 '<PackageReference' {} \; 2>/dev/null
# Check for ProjectReference entries (internal dependencies)
echo "=== ProjectReferences ==="
find . -name "Cuemon.Extensions.Threading.csproj" -exec grep -A 2 '<ProjectReference' {} \; 2>/dev/nullRepository: codebeltnet/cuemon
Length of output: 275
🏁 Script executed:
cat -n src/Cuemon.Extensions.Threading/Cuemon.Extensions.Threading.csprojRepository: codebeltnet/cuemon
Length of output: 746
🏁 Script executed:
# Check the Cuemon.Threading project file to see if it has version info
cat -n src/Cuemon.Threading/Cuemon.Threading.csproj | head -30
# Also check if there are any package version specifications in the solution
rg -i "version.*10\.2" --type xml -A 2 -B 2 | head -50Repository: codebeltnet/cuemon
Length of output: 942
Release notes claim dependency upgrades but this package has no external NuGet dependencies—only internal project references.
The release note states "Dependencies have been upgraded to the latest compatible versions," but the project file shows no <PackageReference> entries, only an internal <ProjectReference> to Cuemon.Threading. The generic wording doesn't clarify what actually changed. Either specify the internal dependency upgrade (e.g., "CHANGED Cuemon.Threading dependency updated to support version 10.2.0 features") or remove the vague claim if no actual dependency changes occurred.
🤖 Prompt for AI Agents
In @.nuget/Cuemon.Extensions.Threading/PackageReleaseNotes.txt around lines 1 -
6, The release note line "CHANGED Dependencies have been upgraded..." is
misleading because this package contains no external PackageReference entries;
update the PackageReleaseNotes.txt to either explicitly name the internal
project dependency (e.g., "CHANGED Cuemon.Threading ProjectReference updated to
X.Y.Z" or "CHANGED Cuemon.Threading updated to 10.2.0") if you actually bumped
the internal reference, or remove/replace the generic dependency claim with a
precise note stating no external NuGet dependencies were changed; reference the
Cuemon.Threading ProjectReference and the PackageReleaseNotes.txt entry when
making the edit.
| @@ -1,3 +1,9 @@ | |||
| Version: 10.2.0 | |||
There was a problem hiding this comment.
Align the version header format.
Line 1 uses Version: 10.2.0 while prior entries use Version 10.1.2 (no colon). Please standardize the header format to avoid tooling/parsing inconsistencies.
🤖 Prompt for AI Agents
In @.nuget/Cuemon.Security.Cryptography/PackageReleaseNotes.txt at line 1,
Standardize the release notes header by removing the colon in the version line
so it matches prior entries (change "Version: 10.2.0" to "Version 10.2.0");
update the header in PackageReleaseNotes.txt to use the same "Version <number>"
format to avoid tooling/parsing inconsistencies.
| foreach (var regionData in regions) | ||
| { | ||
| if (!string.IsNullOrEmpty(regionData.ParentCode) && | ||
| RegionsByCode.TryGetValue(regionData.ParentCode, out var parent)) | ||
| { | ||
| var region = RegionsByCode[regionData.Code]; | ||
| region.Parent = parent; | ||
| parent.AddChild(region); | ||
| } | ||
| } |
| foreach (var countryData in countries) | ||
| { | ||
| if (RegionsByCode.TryGetValue(countryData.ParentCode, out var parent)) | ||
| { | ||
| // Validate kind is CountryOrTerritory | ||
| var kind = ParseKind(countryData.Kind, countryData.Code, countryData.Name); | ||
| if (kind != StatisticalRegionKind.CountryOrTerritory) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Country {countryData.Name} ({countryData.Code}) must have kind 'CountryOrTerritory', but was '{countryData.Kind}'."); | ||
| } | ||
|
|
||
| // Try to find matching RegionInfo | ||
| RegionInfo regionInfo = null; | ||
| try | ||
| { | ||
| regionInfo = World.Regions.FirstOrDefault(r => string.Equals(r.TwoLetterISORegionName, countryData.IsoAlpha2, StringComparison.OrdinalIgnoreCase)); | ||
| } | ||
| catch | ||
| { | ||
| // Some territories may not be supported by the OS | ||
| } | ||
|
|
||
| var country = new StatisticalRegionInfo( | ||
| countryData.Code, | ||
| countryData.Name, | ||
| countryData.IsoAlpha2, | ||
| countryData.IsoAlpha3, | ||
| parent, | ||
| countryData.Ldc, | ||
| countryData.Lldc, | ||
| countryData.Sids, | ||
| regionInfo); | ||
|
|
||
| CountriesByCode[countryData.Code] = country; | ||
| if (!string.IsNullOrEmpty(countryData.IsoAlpha2)) | ||
| { | ||
| CountriesByIsoAlpha2[countryData.IsoAlpha2] = country; | ||
| } | ||
|
|
||
| // Add country as child of its immediate parent region | ||
| parent.AddChild(country); | ||
| } | ||
| } |
| foreach (var region in Regions) | ||
| { | ||
| if (region.Code != "001" && region.Parent == null) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Region {region.Name} ({region.Code}) must have a parent."); | ||
| } | ||
| } |
| catch | ||
| { | ||
| // Some territories may not be supported by the OS | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/Cuemon.Core/Globalization/UnM49DataContainer.cs`:
- Around line 151-194: A country entry is silently skipped when
RegionsByCode.TryGetValue(countryData.ParentCode, out var parent) returns false;
update the loop in UnM49DataContainer so missing parent codes are surfaced by
either throwing an InvalidOperationException or logging an error during
validation: when TryGetValue fails for countryData.ParentCode, include the
offending countryData.Code, countryData.Name and countryData.ParentCode in the
error message (instead of dropping the country), ensuring downstream maps like
CountriesByCode/CountriesByIsoAlpha2 and parent.AddChild are only executed when
a valid parent exists.
- Around line 169-172: The bare catch in UnM49DataContainer (around the
RegionInfo creation loop) should not swallow all exceptions; change it to catch
only the expected exception type(s) (e.g., ArgumentException and possibly
CultureNotFoundException) thrown when a territory/RegionInfo is unsupported, and
handle them explicitly (either ignore or log a warning). Locate the try/catch
inside the method constructing RegionInfo instances in UnM49DataContainer.cs and
replace the catch with specific catches for ArgumentException (and optionally
CultureNotFoundException), ensuring other fatal exceptions bubble up.
- Around line 32-38: The null-check after calling
Decorator.Enclose(typeof(StatisticalRegionInfo).Assembly).GetManifestResources(resourceName).Single().Value
is unreachable because Single() throws if nothing matches; in UnM49DataContainer
change the call to SingleOrDefault() (or FirstOrDefault()) and assign the result
to a variable (e.g., var resource = ...SingleOrDefault()), then keep the
existing null check against resource (or resource?.Value) and throw the
InvalidOperationException if null so the guard is effective; ensure you update
the using(...) to use the resolved resource variable (resource.Value) and
dispose appropriately.
🧹 Nitpick comments (3)
src/Cuemon.Core/Globalization/StatisticalRegionInfo.cs (2)
120-122:Countriesmaterializes a full subtree traversal on every access.
.ToList()forces a complete recursive enumeration and allocation each time the property is read. For the World node this walks ~250 countries per call. Either cache the result (invalidating onAddChild) or drop.ToList()to keep it lazy and let consumers materialize if needed.Option A – keep it lazy
- public IEnumerable<StatisticalRegionInfo> Countries => GetAllDescendants() - .Where(r => r.Kind == StatisticalRegionKind.CountryOrTerritory) - .ToList(); + public IEnumerable<StatisticalRegionInfo> Countries => GetAllDescendants() + .Where(r => r.Kind == StatisticalRegionKind.CountryOrTerritory);
110-110:Childrenexposes the mutable backing list by reference.Callers can cast the returned
IEnumerable<StatisticalRegionInfo>toList<StatisticalRegionInfo>and mutate it, bypassing theAddChildguard. Since the constructors are internal this risk is limited, but wrapping withAsReadOnly()would be safer.Suggested fix
- public IEnumerable<StatisticalRegionInfo> Children => _children; + public IEnumerable<StatisticalRegionInfo> Children => _children.AsReadOnly();src/Cuemon.Core/Globalization/UnM49DataContainer.cs (1)
89-125: Hand-rolled CSV parser — consider noting its limitations.The parser handles quoted fields and escaped double-quotes, which covers standard RFC 4180. Just be aware it doesn't handle newlines embedded within quoted fields. If the CSV data is controlled (embedded resource), this is likely fine, but worth a comment.
| using (var stream = Decorator.Enclose(typeof(StatisticalRegionInfo).Assembly).GetManifestResources(resourceName) | ||
| .Single().Value) | ||
| { | ||
| if (stream == null) | ||
| { | ||
| throw new InvalidOperationException($"Embedded resource '{resourceName}' not found."); | ||
| } |
There was a problem hiding this comment.
Dead code: null check is unreachable after .Single().
.Single() will throw InvalidOperationException if no matching resource is found, so execution never reaches the stream == null check. Use .SingleOrDefault() if you want the null-check path, or remove the null guard.
Suggested fix
- using (var stream = Decorator.Enclose(typeof(StatisticalRegionInfo).Assembly).GetManifestResources(resourceName)
- .Single().Value)
- {
- if (stream == null)
- {
- throw new InvalidOperationException($"Embedded resource '{resourceName}' not found.");
- }
+ var resource = Decorator.Enclose(typeof(StatisticalRegionInfo).Assembly).GetManifestResources(resourceName)
+ .SingleOrDefault();
+ if (resource.Value == null)
+ {
+ throw new InvalidOperationException($"Embedded resource '{resourceName}' not found.");
+ }
+
+ using (var stream = resource.Value)
+ {🤖 Prompt for AI Agents
In `@src/Cuemon.Core/Globalization/UnM49DataContainer.cs` around lines 32 - 38,
The null-check after calling
Decorator.Enclose(typeof(StatisticalRegionInfo).Assembly).GetManifestResources(resourceName).Single().Value
is unreachable because Single() throws if nothing matches; in UnM49DataContainer
change the call to SingleOrDefault() (or FirstOrDefault()) and assign the result
to a variable (e.g., var resource = ...SingleOrDefault()), then keep the
existing null check against resource (or resource?.Value) and throw the
InvalidOperationException if null so the guard is effective; ensure you update
the using(...) to use the resolved resource variable (resource.Value) and
dispose appropriately.
| foreach (var countryData in countries) | ||
| { | ||
| if (RegionsByCode.TryGetValue(countryData.ParentCode, out var parent)) | ||
| { | ||
| // Validate kind is CountryOrTerritory | ||
| var kind = ParseKind(countryData.Kind, countryData.Code, countryData.Name); | ||
| if (kind != StatisticalRegionKind.CountryOrTerritory) | ||
| { | ||
| throw new InvalidOperationException( | ||
| $"Country {countryData.Name} ({countryData.Code}) must have kind 'CountryOrTerritory', but was '{countryData.Kind}'."); | ||
| } | ||
|
|
||
| // Try to find matching RegionInfo | ||
| RegionInfo regionInfo = null; | ||
| try | ||
| { | ||
| regionInfo = World.Regions.FirstOrDefault(r => string.Equals(r.TwoLetterISORegionName, countryData.IsoAlpha2, StringComparison.OrdinalIgnoreCase)); | ||
| } | ||
| catch | ||
| { | ||
| // Some territories may not be supported by the OS | ||
| } | ||
|
|
||
| var country = new StatisticalRegionInfo( | ||
| countryData.Code, | ||
| countryData.Name, | ||
| countryData.IsoAlpha2, | ||
| countryData.IsoAlpha3, | ||
| parent, | ||
| countryData.Ldc, | ||
| countryData.Lldc, | ||
| countryData.Sids, | ||
| regionInfo); | ||
|
|
||
| CountriesByCode[countryData.Code] = country; | ||
| if (!string.IsNullOrEmpty(countryData.IsoAlpha2)) | ||
| { | ||
| CountriesByIsoAlpha2[countryData.IsoAlpha2] = country; | ||
| } | ||
|
|
||
| // Add country as child of its immediate parent region | ||
| parent.AddChild(country); | ||
| } | ||
| } |
There was a problem hiding this comment.
Countries with an unrecognized ParentCode are silently dropped.
If TryGetValue fails at line 153, the country is skipped with no warning. A data error in the CSV (e.g., a typo in a parent code) would silently omit a country from the hierarchy. Consider logging or throwing during validation to surface data integrity issues.
🤖 Prompt for AI Agents
In `@src/Cuemon.Core/Globalization/UnM49DataContainer.cs` around lines 151 - 194,
A country entry is silently skipped when
RegionsByCode.TryGetValue(countryData.ParentCode, out var parent) returns false;
update the loop in UnM49DataContainer so missing parent codes are surfaced by
either throwing an InvalidOperationException or logging an error during
validation: when TryGetValue fails for countryData.ParentCode, include the
offending countryData.Code, countryData.Name and countryData.ParentCode in the
error message (instead of dropping the country), ensuring downstream maps like
CountriesByCode/CountriesByIsoAlpha2 and parent.AddChild are only executed when
a valid parent exists.
| catch | ||
| { | ||
| // Some territories may not be supported by the OS | ||
| } |
There was a problem hiding this comment.
Bare catch swallows all exceptions silently.
This catches everything, including OutOfMemoryException, ThreadAbortException, etc. If the intent is to tolerate missing OS region support, catch the specific exception type (e.g., ArgumentException thrown by RegionInfo).
Suggested fix
- catch
+ catch (ArgumentException)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| catch | |
| { | |
| // Some territories may not be supported by the OS | |
| } | |
| catch (ArgumentException) | |
| { | |
| // Some territories may not be supported by the OS | |
| } |
🤖 Prompt for AI Agents
In `@src/Cuemon.Core/Globalization/UnM49DataContainer.cs` around lines 169 - 172,
The bare catch in UnM49DataContainer (around the RegionInfo creation loop)
should not swallow all exceptions; change it to catch only the expected
exception type(s) (e.g., ArgumentException and possibly
CultureNotFoundException) thrown when a territory/RegionInfo is unsupported, and
handle them explicitly (either ignore or log a warning). Locate the try/catch
inside the method constructing RegionInfo instances in UnM49DataContainer.cs and
replace the catch with specific catches for ArgumentException (and optionally
CultureNotFoundException), ensuring other fatal exceptions bubble up.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
==========================================
+ Coverage 80.37% 80.53% +0.15%
==========================================
Files 595 598 +3
Lines 18485 18826 +341
Branches 1895 1934 +39
==========================================
+ Hits 14857 15161 +304
- Misses 3562 3599 +37
Partials 66 66 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request introduces support for UN M.49 statistical regions and countries in the
Cuemon.Corepackage, enabling standardized geographic hierarchy and country metadata. It also updates copyright years and package tags, and adds a new guide for agentic coding tools.UN M.49 Statistical Region Support
StatisticalRegionInfoandStatisticalRegionKindinCuemon.Globalization, representing the UN M.49 hierarchy of regions and countries, including country metadata and hierarchy traversal methods. [1] [2]Cuemon.Core.csprojto embed UN M.49 data (unm49-data.json), add relevant package tags, and referenceSystem.Text.Jsonfor netstandard2.0.Documentation and Metadata Updates
AGENTS.mdat repo root to guide agentic coding tools, detailing repo structure, build/test commands, conventions, and Copilot rules.LICENSE.mdand.docfx/docfx.json. [1] [2]Summary by CodeRabbit
New Features
Changed
Documentation