Skip to content

style: idiomatic cleanups in metadata build tooling - #413

Merged
twcclegg merged 1 commit into
mainfrom
style/metadata-builders
Aug 26, 2026
Merged

style: idiomatic cleanups in metadata build tooling#413
twcclegg merged 1 commit into
mainfrom
style/metadata-builders

Conversation

@twcclegg

Copy link
Copy Markdown
Owner

Summary

Small readability pass over BuildMetadataFromXml.cs, part of a broader sweep to bring the C# port's style up to date file-by-file (same spirit as #409).

  • ValidateRE: replaced the manual whitespace-stripping char loop with a LINQ filter.
  • ArePossibleLengthsEqual: collapsed the manual index loop into a single SequenceEqual call against PhoneNumberDesc.PossibleLengthList (both sides are already sorted, per the existing comment).
  • is null / is not null instead of == null / != null throughout.

BuildMetadataFromBin.cs was reviewed too but deliberately left untouched — its field-by-field binary read/write order is explicit and order-sensitive by design (see its own doc comment), and "idiomatic" restructuring there is a correctness risk for no real readability gain.

No behavior change.

Test plan

  • dotnet build csharp/PhoneNumbers.slnx — 0 warnings, 0 errors
  • dotnet test csharp/PhoneNumbers.slnx — 414/414 passing on net8.0 and net10.0

- Replace the manual whitespace-stripping char loop in ValidateRE with
  a LINQ filter.
- Simplify ArePossibleLengthsEqual to a single SequenceEqual call
  against PhoneNumberDesc.PossibleLengthList.
- Use is null / is not null instead of == null / != null throughout.

No behavior change; BuildMetadataFromBin.cs was reviewed but left
untouched since its field-by-field read/write order is deliberately
explicit and order-sensitive.
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.45%. Comparing base (e75ae83) to head (65c3c56).

Files with missing lines Patch % Lines
csharp/PhoneNumbers/BuildMetadataFromXml.cs 86.66% 0 Missing and 2 partials ⚠️

❌ Your patch check has failed because the patch coverage (86.66%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #413      +/-   ##
==========================================
+ Coverage   87.42%   87.45%   +0.02%     
==========================================
  Files          41       41              
  Lines        3856     3841      -15     
  Branches      990      983       -7     
==========================================
- Hits         3371     3359      -12     
+ Misses        283      281       -2     
+ Partials      202      201       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown

📊 Benchmark Results

Commit: 65c3c56 · Full run · Linux ubuntu-24.04-arm

Both sides were measured on the same runner in the same job, so the numbers are
comparable. Treat sub-percent differences as noise.

PR branch

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
InputDigitPerKeystroke 1000 4.313 ms 0.0106 ms 0.0100 ms 54.6875 3.88 MB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]     : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  Job-AMQORM : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Runtime=.NET 10.0  InvocationCount=1  IterationCount=20  
LaunchCount=1  RunStrategy=ColdStart  UnrollFactor=1  
WarmupCount=1  

Method Mean Error StdDev Allocated
CreateInstance 402.2 μs 113.7 μs 130.9 μs 119.48 KB
CreateInstanceAndLoadAllRegions 7,086.6 μs 394.3 μs 454.1 μs 1620.34 KB
FirstRegionLookup 448.1 μs 132.6 μs 152.7 μs 124.54 KB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
ExtractPossibleNumber_CleanInput 1000 20.91 μs 0.019 μs 0.018 μs - -
ExtractPossibleNumber_WithLeadingJunk 1000 37.46 μs 0.053 μs 0.049 μs 0.6714 48360 B

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
FindNumbers_Valid 100 143.6 μs 0.27 μs 0.24 μs 0.9766 70.71 KB
FindNumbers_StrictGrouping 100 325.5 μs 4.74 μs 4.20 μs 1.4648 124.84 KB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
GetDescriptionForNumber 1000 1,359.11 μs 8.717 μs 8.154 μs 1.9531 196.59 KB
GetDisplayCountry 1000 18.57 μs 0.031 μs 0.028 μs 0.0916 7.56 KB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
ParseValidateAndFormatPhoneNumbers 1000 2,501.9 μs 4.56 μs 4.04 μs 7.8125 577.71 KB
ParseOnly 1000 437.6 μs 0.49 μs 0.46 μs 4.8828 348.13 KB
ParseNationalFormat 1000 761.7 μs 0.67 μs 0.52 μs 5.8594 431.36 KB
ParseWithExtension 1000 1,113.8 μs 9.86 μs 9.22 μs 13.6719 957.38 KB
ValidateOnly 1000 796.0 μs 0.72 μs 0.63 μs - 41.11 KB
FormatOnly 1000 1,117.7 μs 1.57 μs 1.39 μs 1.9531 187.92 KB
PR base

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
InputDigitPerKeystroke 1000 4.401 ms 0.0072 ms 0.0067 ms 54.6875 3.88 MB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]     : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  Job-AMQORM : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Runtime=.NET 10.0  InvocationCount=1  IterationCount=20  
LaunchCount=1  RunStrategy=ColdStart  UnrollFactor=1  
WarmupCount=1  

Method Mean Error StdDev Allocated
CreateInstance 386.1 μs 102.2 μs 117.7 μs 119.48 KB
CreateInstanceAndLoadAllRegions 7,026.1 μs 357.2 μs 411.4 μs 1620.34 KB
FirstRegionLookup 462.2 μs 144.8 μs 166.8 μs 124.54 KB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
ExtractPossibleNumber_CleanInput 1000 20.87 μs 0.021 μs 0.019 μs - -
ExtractPossibleNumber_WithLeadingJunk 1000 38.24 μs 0.093 μs 0.087 μs 0.6714 48360 B

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
FindNumbers_Valid 100 140.5 μs 0.34 μs 0.32 μs 0.9766 70.71 KB
FindNumbers_StrictGrouping 100 313.4 μs 0.57 μs 0.53 μs 1.4648 124.84 KB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
GetDescriptionForNumber 1000 1,367.87 μs 14.681 μs 13.732 μs 1.9531 196.59 KB
GetDisplayCountry 1000 18.63 μs 0.019 μs 0.016 μs 0.0916 7.56 KB

BenchmarkDotNet v0.15.8, Linux Ubuntu 24.04.4 LTS (Noble Numbat)
Neoverse-N2, 4 physical cores
.NET SDK 10.0.400
  [Host]    : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a
  .NET 10.0 : .NET 10.0.11 (10.0.11, 10.0.1126.37416), Arm64 RyuJIT armv8.0-a

Job=.NET 10.0  Runtime=.NET 10.0  

Method PhoneNumberCount Mean Error StdDev Gen0 Allocated
ParseValidateAndFormatPhoneNumbers 1000 2,644.8 μs 2.74 μs 2.14 μs 7.8125 577.71 KB
ParseOnly 1000 441.6 μs 0.61 μs 0.57 μs 4.8828 348.13 KB
ParseNationalFormat 1000 785.3 μs 0.95 μs 0.89 μs 5.8594 431.36 KB
ParseWithExtension 1000 1,112.7 μs 1.96 μs 1.84 μs 13.6719 957.38 KB
ValidateOnly 1000 797.7 μs 1.02 μs 0.85 μs - 41.11 KB
FormatOnly 1000 1,128.6 μs 1.87 μs 1.66 μs 1.9531 187.92 KB

@twcclegg
twcclegg merged commit eb5f0e8 into main Aug 26, 2026
7 of 8 checks passed
@twcclegg
twcclegg deleted the style/metadata-builders branch August 26, 2026 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant