Add IQuantity.UnitInfo convenience property#1649
Conversation
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) <noreply@anthropic.com>
PR Review: Add IQuantity.UnitInfo convenience propertyThis is a small, focused change adding a shorthand for Potential breaking changeAdding API asymmetry (expected, but worth documenting)Callers on different TFMs need different call sites:
Consider adding a Typed variant gap
Test coverageNo new tests. Given this is a one-liner wrapper it's not strictly required, but a test asserting |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR Review: Add IQuantity.UnitInfo convenience propertyOverall this is a nice quality-of-life addition. A few things to address: API Asymmetry Between TargetsThe two APIs differ by more than the target framework — they have different names and calling conventions:
Any library or user code that targets both frameworks must use Missing Tests for netstandard2.0 PathThe two new tests in Minor: Potential Conflict with ImplementorsAdding a default interface member named Looks Good
|
- 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) <noreply@anthropic.com>
PR Review: Add IQuantity.UnitInfo convenience propertyNice, focused ergonomics improvement. Implementation follows existing What looks good
Missing test coverage for netstandard2.0 path
API fragmentation across TFMsCallers targeting both netstandard2.0 and net5+ must still write Minor: potential shadowing by implementorsAny custom |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ReviewThis is a clean, well-motivated PR. The implementation looks correct and consistent with existing Design: API asymmetry on netstandard2.0On .NET 5+, 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];
}
#endifThis would match the typed Default interface member with explicit bridge — correct patternThe TestsCoverage is solid — tests cover non-generic, typed, zero, and cross-all-quantities assertions. Minor note: No breaking changesAdding a default interface member to Overall a nice ergonomic improvement. The main suggestion is the typed |
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review: Add IQuantity.UnitInfo convenience propertyClean, well-scoped API addition. The DIM + extension method split follows the existing pattern in this codebase (e.g. Potential breaking changeAdding a DIM named Extension methods are untestedThe Minor:
|
|
@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 My second argument against such a property on the interface, is that this implies that if my extended My last argument against this (although I haven't verified it) is that by not implementing the property on the structs, using something like |
|
@angularsen Frankly, I would have probably removed the PS That's not a crazy idea, but a lot needs to be done if we want to get there (not counting the serialization). |
|
Yeah I agree. I just noticed I reverted the new interface members and reused extension methods here as a first step: |
…TFMs (#1657) ## Summary Reverts the default interface members added in #1649 and exposes `GetUnitInfo()` as extension methods on all target frameworks instead. The typed overload returns the most specific `UnitInfo<TQuantity, TUnit>` when overload resolution can infer both type parameters from the receiver. ## Rationale Per @lipchev's [review feedback on #1649](#1649 (comment)): - **Lean interface.** The DIM is just a getter over `QuantityInfo[UnitKey]` — no new state, sets a precedent for further interface bloat (`Dimensions` etc.). - **Semantic trap.** A custom `IQuantity` implementor overriding `UnitInfo` would silently diverge from internal lookups (`UnitConverter`, `UnitAbbreviationsCache`, `QuantityFormatter`) which all go through `QuantityInfo`, not the new property. - **Boxing on structs.** Calling `Mass.Zero.UnitInfo` through `IQuantity` on a struct quantity boxes the receiver per call; concrete quantities don't override the property so the DIM path is the only one available. The extension method gives the same call-site ergonomics (`quantity.GetUnitInfo()`) on every TFM with none of these issues. ## Overload resolution | Caller has | Resolved overload | Return type | |--------------------|-------------------|-------------| | `Length q;` | `GetUnitInfo<TQuantity, TUnit>(IQuantity<TQuantity, TUnit>)` | `UnitInfo<Length, LengthUnit>` | | `IQuantity q;` | `GetUnitInfo(IQuantity)` | `UnitInfo` | | `IQuantity<TUnit> q;` | `GetUnitInfo(IQuantity)` (fallback) | `UnitInfo` | The `IQuantity<TUnit>` reference case loses its typed return — `IQuantity<TUnit>` does not satisfy the `IQuantity<TQuantity, TUnit>` constraint, so resolution falls back to the non-generic overload. That scenario is uncommon compared to the concretely-typed receiver case, which is now strictly better than before (returns `UnitInfo<TQuantity, TUnit>` instead of `UnitInfo<TUnit>`). ## Follow-up PR #1658 stacks on top of this branch: adds `static abstract Info` on `IQuantityOfType<T>` / `IQuantity<TSelf, TUnitType>`, obsoletes the instance `QuantityInfo` member, and provides matching `GetQuantityInfo()` extensions. Out of scope for this PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
IQuantity.UnitInfodefault interface member on .NET 5+ targets, so consumers can writequantity.UnitInfoinstead ofquantity.QuantityInfo[quantity.UnitKey]IQuantity<TUnitType>.UnitInforeturningUnitInfo<TUnitType>on .NET 5+GetUnitInfo()andGetUnitInfo<TUnit>()extension methods onnetstandard2.0for the same functionalityMotivation
Getting
UnitInfofrom a quantity instance currently requires the verbosequantity.QuantityInfo[quantity.UnitKey]. This is a common operation, especially when chaining with APIs likeUnitAbbreviationsCachethat benefit from havingUnitInfodirectly (see #1067).Design decisions
TFM fragmentation
The extension methods are gated behind
#if !NETso that .NET 5+ consumers only see the cleaner property syntax in IntelliSense. Library authors who multi-target can usequantity.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#ifguard is a non-breaking one-line change.Extension method tests
The test project targets
net8.0;net9.0;net10.0only, so#if !NETcode 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 frameworksUnitInfo_ReturnsUnitInfoForQuantityUnit— non-base unit viaIQuantityUnitInfo_Zero_ReturnsBaseUnitInfo—.ZeroinstanceUnitInfo_TypedQuantity_ReturnsTypedUnitInfo— typedIQuantity<LengthUnit>UnitInfo_MatchesUnit—Assert.Allacross all quantities🤖 Generated with Claude Code