Skip to content

Add IQuantity.UnitInfo convenience property#1649

Merged
angularsen merged 5 commits intomasterfrom
agl/iquantity-unitinfo-property
Apr 6, 2026
Merged

Add IQuantity.UnitInfo convenience property#1649
angularsen merged 5 commits intomasterfrom
agl/iquantity-unitinfo-property

Conversation

@angularsen
Copy link
Copy Markdown
Owner

@angularsen angularsen commented Apr 6, 2026

Summary

  • Adds IQuantity.UnitInfo default interface member on .NET 5+ targets, so consumers can write quantity.UnitInfo instead of quantity.QuantityInfo[quantity.UnitKey]
  • Adds typed IQuantity<TUnitType>.UnitInfo returning UnitInfo<TUnitType> on .NET 5+
  • Adds GetUnitInfo() and GetUnitInfo<TUnit>() extension methods on netstandard2.0 for the same functionality

Motivation

Getting UnitInfo from a quantity instance currently requires the verbose quantity.QuantityInfo[quantity.UnitKey]. This is a common operation, especially when chaining with APIs like UnitAbbreviationsCache that benefit from having UnitInfo directly (see #1067).

Design decisions

TFM fragmentation

The extension methods are gated behind #if !NET so that .NET 5+ consumers only see the cleaner property syntax in IntelliSense. Library authors who multi-target can use quantity.QuantityInfo[quantity.UnitKey] which works on all targets, or write a one-line wrapper. If this turns out to be a real pain point, removing the #if guard is a non-breaking one-line change.

Extension method tests

The test project targets net8.0;net9.0;net10.0 only, so #if !NET code is never compiled in tests. The extension methods are trivial one-liner wrappers over the same indexer call that the default interface members use, so they are effectively covered by the property tests.

Test plan

  • dotnet build UnitsNet/UnitsNet.csproj — 0 errors across all 4 target frameworks
  • New tests pass on net8.0, net9.0, net10.0:
    • UnitInfo_ReturnsUnitInfoForQuantityUnit — non-base unit via IQuantity
    • UnitInfo_Zero_ReturnsBaseUnitInfo.Zero instance
    • UnitInfo_TypedQuantity_ReturnsTypedUnitInfo — typed IQuantity<LengthUnit>
    • UnitInfo_MatchesUnitAssert.All across all quantities

🤖 Generated with Claude Code

Adds a way to get UnitInfo directly from a quantity instance, avoiding
the verbose `quantity.QuantityInfo[quantity.UnitKey]` pattern.

On net8.0+ this is a default interface member (`quantity.UnitInfo`).
On netstandard2.0 it is an extension method (`quantity.GetUnitInfo()`).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@claude
Copy link
Copy Markdown

claude bot commented Apr 6, 2026

PR Review: Add IQuantity.UnitInfo convenience property

This is a small, focused change adding a shorthand for quantity.QuantityInfo[quantity.UnitKey]. The implementation follows existing patterns in IQuantity.cs (e.g., the #if NET default interface members on lines 102–118).

Potential breaking change

Adding UnitInfo as a default interface member on IQuantity could conflict with custom IQuantity implementations that already define a UnitInfo property — the compiler would see ambiguity between the interface default and the concrete member. Worth noting in release notes.

API asymmetry (expected, but worth documenting)

Callers on different TFMs need different call sites:

  • net8.0+: quantity.UnitInfo (property)
  • netstandard2.0: quantity.GetUnitInfo() (extension method)

Consider adding a <remarks> to the extension method pointing to the property equivalent on newer targets, for discoverability.

Typed variant gap

IQuantity<TUnitType> doesn't override UnitInfo with a typed UnitInfo<TUnitType> variant. Callers with a strongly-typed IQuantity<LengthUnit> get back the untyped UnitInfo rather than UnitInfo<Length, LengthUnit>. Probably intentional, but if a typed version would be useful it could be added here.

Test coverage

No new tests. Given this is a one-liner wrapper it's not strictly required, but a test asserting quantity.GetUnitInfo().Value == quantity.Unit (or property equivalent) would serve as documentation and catch regressions.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@claude
Copy link
Copy Markdown

claude bot commented Apr 6, 2026

PR Review: Add IQuantity.UnitInfo convenience property

Overall this is a nice quality-of-life addition. A few things to address:

API Asymmetry Between Targets

The two APIs differ by more than the target framework — they have different names and calling conventions:

  • .NET 5+: quantity.UnitInfo (property)
  • netstandard2.0: quantity.GetUnitInfo() (extension method)

Any library or user code that targets both frameworks must use #if NET just to call this, which defeats the ergonomics goal. Consider removing the #if !NET guard on the extension method so GetUnitInfo() is available on all targets. Then callers can write quantity.GetUnitInfo() regardless of TFM, and .NET 5+ callers who prefer property syntax can use quantity.UnitInfo. The duplication of logic is trivial (it's a one-liner).

Missing Tests for netstandard2.0 Path

The two new tests in IQuantityTests.cs both use quantity.UnitInfo, which only compiles on .NET 5+. The GetUnitInfo() extension method has no test coverage at all. A test for GetUnitInfo() should be added (likely guarded with #if !NET or by testing it unconditionally if available on all targets as suggested above).

Minor: Potential Conflict with Implementors

Adding a default interface member named UnitInfo to IQuantity is technically non-breaking since implementors don't need to provide it. However, any existing class that implements IQuantity and already exposes a member named UnitInfo (e.g., a custom quantity type) will now shadow the DIM silently. Worth a note in the summary/changelog.

Looks Good

  • The DIM implementation QuantityInfo[UnitKey] is consistent with how it's used throughout the codebase.
  • Use of #if NET is consistent with the existing pattern in IQuantity.cs.
  • Doc comments are clear.
  • No generated code changes needed (this is hand-written API surface only).

- Add typed UnitInfo<TUnitType> on IQuantity<TUnitType> for strongly-typed access
- Add <remarks> cross-referencing the property and extension method
- Add test for typed IQuantity<LengthUnit>.UnitInfo
- Add test asserting UnitInfo.Value matches Unit across all quantities

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@claude
Copy link
Copy Markdown

claude bot commented Apr 6, 2026

PR Review: Add IQuantity.UnitInfo convenience property

Nice, focused ergonomics improvement. Implementation follows existing #if NET DIM patterns in IQuantity.cs.

What looks good

  • DIM implementation QuantityInfo[UnitKey] is consistent throughout the codebase
  • Typed override UnitInfo<TUnitType> is correctly added in IQuantity<TUnitType> with the explicit interface impl bridging to the base
  • 4 tests added covering the untyped property, base unit case, typed variant (IQuantity<LengthUnit>), and all quantities
  • No generated code changes needed — this is hand-written API surface only
  • #if NET / #if !NET guards are consistent with the existing pattern

Missing test coverage for netstandard2.0 path

GetUnitInfo() extension method (the netstandard2.0 fallback) has no test. Since the tests use quantity.UnitInfo which only compiles under #if NET, the netstandard2.0 code path is untested. A simple test calling quantity.GetUnitInfo() would close this gap.

API fragmentation across TFMs

Callers targeting both netstandard2.0 and net5+ must still write #if NET at every call site to use the property vs. extension method. A small mitigation: make GetUnitInfo() available on all targets (remove the #if !NET guard). It is a one-liner with negligible cost, and cross-TFM code can then consistently use quantity.GetUnitInfo() without ifdefs. The property on .NET 5+ remains available for those who prefer it.

Minor: potential shadowing by implementors

Any custom IQuantity implementation that already defines a member named UnitInfo will silently shadow the new DIM. Not a runtime bug, but worth a note in the changelog.

@claude
Copy link
Copy Markdown

claude bot commented Apr 6, 2026

Review

This is a clean, well-motivated PR. The implementation looks correct and consistent with existing #if NET patterns in the codebase. A few observations:

Design: API asymmetry on netstandard2.0

On .NET 5+, IQuantity<TUnitType>.UnitInfo returns the typed UnitInfo<TUnitType>. On netstandard2.0, the GetUnitInfo() extension only overloads IQuantity (non-generic), returning the non-generic UnitInfo. Consumers on netstandard2.0 with an IQuantity<LengthUnit> would have to cast manually to get back UnitInfo<LengthUnit>.

Consider adding a typed overload for netstandard2.0 parity:

#if !NET
public static UnitInfo<TUnit> GetUnitInfo<TUnit>(this IQuantity<TUnit> quantity)
    where TUnit : struct, Enum
{
    return quantity.QuantityInfo[quantity.Unit];
}
#endif

This would match the typed UnitInfo<TUnitType> return that .NET 5+ consumers get via the DIM.

Default interface member with explicit bridge — correct pattern

The new UnitInfo<TUnitType> UnitInfo in IQuantity<TUnitType> hiding the base + the explicit UnitInfo IQuantity.UnitInfo => UnitInfo bridge is the right approach for interface covariance here. Looks good.

Tests

Coverage is solid — tests cover non-generic, typed, zero, and cross-all-quantities assertions. Minor note: UnitInfo_MatchesUnit uses Quantity.Infos.Select(x => x.Zero) which always yields the base unit. A variant with a non-base unit (e.g. new Length(1, LengthUnit.Centimeter)) would give extra confidence that the property works beyond the base-unit case.

No breaking changes

Adding a default interface member to IQuantity is non-breaking for existing implementors (they get the default implementation). ✓

Overall a nice ergonomic improvement. The main suggestion is the typed GetUnitInfo overload for full netstandard2.0 parity.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96%. Comparing base (414fe57) to head (716bca9).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
UnitsNet/IQuantity.cs 66% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1649   +/-   ##
======================================
- Coverage      96%     96%   -1%     
======================================
  Files         450     450           
  Lines       29148   29151    +3     
======================================
+ Hits        28107   28109    +2     
- Misses       1041    1042    +1     

☔ View full report in Codecov by Sentry.
📢 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.

@claude
Copy link
Copy Markdown

claude bot commented Apr 6, 2026

Review: Add IQuantity.UnitInfo convenience property

Clean, well-scoped API addition. The DIM + extension method split follows the existing pattern in this codebase (e.g. IQuantity<TSelf, TUnitType>.From, Create) and the design rationale is well explained.

Potential breaking change

Adding a DIM named UnitInfo to IQuantity on .NET 5+ is a source-breaking change for any external implementor that already has a UnitInfo member with an incompatible signature. This is very low probability, but worth a changelog/release notes entry.

Extension methods are untested

The #if !NET extension methods (GetUnitInfo / GetUnitInfo<TUnit>) are dead code in the test suite because all test TFMs are net8.0+. The PR description acknowledges this and the logic is trivially simple (one-liner wrappers over the same indexer). Acceptable as-is, but adding a comment in the test class noting the gap would help the next person.

Minor: UnitInfo_MatchesUnit only covers base units

Assert.All(Quantity.Infos.Select(x => x.Zero), quantity =>
{
    Assert.Equal(quantity.Unit, quantity.UnitInfo.Value);
});

Info.Zero always returns a base-unit quantity, so this only exercises the base-unit path. A second sweep over a non-base unit per quantity would give stronger coverage.

Minor: XML doc scope for netstandard2.1

The remarks say "targets that do not support default interface members (e.g. netstandard2.0)". netstandard2.1 also hits the #if !NET path (since #if NET is NET5_0_OR_GREATER), even though it does support DIMs. The code is correct — the remark just slightly understates the scope.

Overall

The explicit bridge UnitInfo IQuantity.UnitInfo => UnitInfo; in IQuantity<TUnitType> correctly satisfies the base interface for all typed implementors without requiring them to do anything, and the new keyword on the typed override is properly placed. The four tests cover the meaningful cases (non-base unit, zero, typed interface, all-quantities sweep).

@angularsen angularsen enabled auto-merge (squash) April 6, 2026 12:40
@angularsen angularsen merged commit 2a60bf9 into master Apr 6, 2026
5 of 6 checks passed
@angularsen angularsen deleted the agl/iquantity-unitinfo-property branch April 6, 2026 12:58
@lipchev
Copy link
Copy Markdown
Collaborator

lipchev commented Apr 6, 2026

@angularsen I though you wanted a lean interface.. Introducing two interface members that are nothing more than getters from an existing property (which is explicitly implemented) feels like going in the direction of having a Dimensions property on the interface.

My second argument against such a property on the interface, is that this implies that if my extended IQuantity implements the UnitInfo properties in some other way, something will behave differently. This wouldn't be the case- all types such as the UnitAbbreviationsCache the UnitConverter etc, are all oblivious of the existence of such a property (and in fact expect that the UnitInfo is inseparable from the QuantityInfo that was used in the constructor).

My last argument against this (although I haven't verified it) is that by not implementing the property on the structs, using something like Mass.Zero.UnitInfo is likely going to incur an allocation cost (because of the interface dispatch of the struct).

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.

2 participants