Add scoped UnitInfo overloads to UnitAbbreviationsCache

[bdbonazz]

Summary

Adds UnitInfo convenience overloads to UnitAbbreviationsCache for the same method groups that already accept UnitKey:

• GetDefaultAbbreviation(UnitInfo, ...)
• GetUnitAbbreviations(UnitInfo, ...)
• MapUnitToAbbreviation(UnitInfo, ...)
• MapUnitToDefaultAbbreviation(UnitInfo, ...)

This avoids caller-side unitInfo.UnitKey extraction when the caller already has a UnitInfo, while keeping the existing UnitKey, typed enum, and (Type, int) overloads.

Maintainer Updates

• Added explicit ArgumentNullException(nameof(unitInfo)) handling for the new public overloads.
• Kept the overloads scoped to the cache's configured quantities by resolving the supplied UnitInfo.UnitKey through the cache's QuantityInfoLookup before reading or mapping abbreviations.
• Added tests for null UnitInfo inputs.
• Added tests confirming a cache constructed with a limited quantity set rejects UnitInfo values outside that set.

Validation

• Ran dotnet test UnitsNet.Tests --filter "FullyQualifiedName~UnitAbbreviationsCacheTests" --no-restore --verbosity minimal.
• Result: 36 tests passed per framework across net8, net9, and net10.

[claude]

Code Review

Summary

Adds UnitInfo overloads to MapUnitToAbbreviation, MapUnitToDefaultAbbreviation, GetDefaultAbbreviation, and GetUnitAbbreviations. The motivation is solid: callers who already hold a UnitInfo (e.g., after iterating QuantityInfo.UnitInfos) shouldn't need to go through a UnitKey → GetUnitInfo() roundtrip. The internal refactoring (delegating the UnitKey path through the new UnitInfo path) is a nice cleanup too.

XML doc typo — double >>

Three of the inheritdoc tags have an extra >:

/// <inheritdoc cref="MapUnitToAbbreviation{TUnitType}(TUnitType,IEnumerable{string})"/>>
/// <inheritdoc cref="MapUnitToDefaultAbbreviation{TUnitType}(TUnitType,string)"/>>

The closing " already ends the attribute; the trailing > is a stray character. Should be "/>.

Missing blank line

There is no blank line between the closing } of GetUnitAbbreviations(UnitKey, ...) and the /// <summary> of the new GetUnitAbbreviations(UnitInfo, ...) overload. The rest of the file separates method overloads with a blank line.

Tests

No tests cover the new overloads. At minimum, a happy-path test like:

UnitInfo massKg = Mass.Info[MassUnit.Kilogram];
string abbrev = UnitAbbreviationsCache.Default.GetDefaultAbbreviation(massKg);
Assert.Equal("kg", abbrev);

...would verify the wiring is correct and guard against accidental regression.

Overall

The direction is right. Fix the XML typos and add a couple of tests and this is ready.

[claude]

Code Review

Overview: Adds UnitInfo overloads to four methods in UnitAbbreviationsCache (MapUnitToAbbreviation, MapUnitToDefaultAbbreviation, GetDefaultAbbreviation, GetUnitAbbreviations) and refactors the existing UnitKey overloads to delegate to the new ones. This avoids redundant Quantities.GetUnitInfo() lookups when a caller already holds a UnitInfo.

✅ What's good

• Correct motivation: avoids a redundant dictionary lookup when UnitInfo is already in hand
• The refactoring direction is correct (old UnitKey overloads now delegate to new UnitInfo overloads)
• Error messages in GetDefaultAbbreviation improve from UnitEnumType.Name to the more readable QuantityInfo.Name + UnitInfo.Name

🐛 Minor issues

1. Double > in XML doc comments — Several <inheritdoc> tags have a trailing extra >:

/// <inheritdoc cref="MapUnitToAbbreviation{TUnitType}(TUnitType,IEnumerable{string})"/>>
//                                                                                    ^^^

This is valid XML but looks like a typo; the extra > is unusual.

2. Missing blank line before GetUnitAbbreviations(UnitInfo ...) overload — The new overload added after GetUnitAbbreviations(UnitKey ...) is missing a preceding blank line, making the code harder to read.

3. Null handling — The UnitKey overloads throw UnitNotFoundException if the key isn't found. The new UnitInfo overloads accept a non-nullable UnitInfo but don't validate for null. Adding a null check with ArgumentNullException would be consistent with .NET conventions.

4. XML doc for GetUnitAbbreviations(UnitInfo ...) param — The <param name="unitInfo"> description says "The key representing the unit info and value" — "key" is a copy-paste artifact from the UnitKey version. Update to "The unit info object representing the unit."

Overall

Good API addition that reduces unnecessary lookups. Fix the minor issues and this is ready.

[claude]

Code Review

Overview

Adds UnitInfo-based overloads to MapUnitToAbbreviation, MapUnitToDefaultAbbreviation, GetDefaultAbbreviation, and GetUnitAbbreviations, and routes the existing UnitKey-based overloads through them to reduce duplication.

Code Quality

The refactoring is clean. Routing UnitKey overloads through UnitInfo overloads (via Quantities.GetUnitInfo(unitKey)) reduces duplication. The improved error message in GetDefaultAbbreviation (quantity.Name + unit.Name instead of the raw enum type/value) is a nice improvement.

Minor Issues

1. Double > in inheritdoc: Several <inheritdoc> tags have a trailing extra >:

   /// <inheritdoc cref="MapUnitToAbbreviation{TUnitType}(TUnitType,IEnumerable{string})"/>>
   

   Should be /> not />>.

2. Missing blank line: The new GetUnitAbbreviations(UnitInfo) overload is missing a blank line before its XML doc comment after the preceding method closing brace.

3. Extra Quantities.GetUnitInfo(unitKey) call: The UnitKey overloads now do an extra dictionary lookup compared to before. This is negligible for most use cases but may matter in tight loops on older TFMs. Not blocking, just worth noting.

Tests

No tests are added. The refactoring is low-risk since behavior is preserved, but a test confirming the UnitInfo overloads work end-to-end would be helpful.

Summary

Good API addition. Fix the double-> typos in the doc comments before merging.

[claude]

Code Review

Adding UnitInfo overloads to UnitAbbreviationsCache — this is a nice improvement. When the caller already has a UnitInfo instance (e.g. from iterating QuantityInfo.UnitInfos) the new overloads avoid the redundant Quantities.GetUnitInfo(unitKey) lookup.

Looks good

• Refactoring PerformAbbreviationMapping away in favour of AddAbbreviation is cleaner.
• The existing UnitKey overloads now delegate to the new UnitInfo overloads — correct layering.
• Error message in GetDefaultAbbreviation is now more readable (QuantityName.UnitName vs the old enum type + cast).

Issues

1. Typo in XML doc comments — double >.
   Both MapUnitToAbbreviation and MapUnitToDefaultAbbreviation have:

   /// <inheritdoc cref="..."/>>
   

   The closing tag has an extra >. Should be /// <inheritdoc cref="..."/>.

2. Stale <param> description.
   In GetUnitAbbreviations(UnitInfo ...):

   /// <param name="unitInfo">The key representing the unit info and value.</param>

   "key" is from the old UnitKey doc; should read "The info representing the unit."

3. Missing blank line before GetUnitAbbreviations(UnitInfo ...). There is no blank line between the closing brace of GetUnitAbbreviations(UnitKey ...) and the XML doc comment of the new overload, which is inconsistent with the rest of the file.

No functional concerns — just polish needed on the doc comments.

[claude]

Code Review

Overview

Adds UnitInfo-based overloads to four methods in UnitAbbreviationsCache: MapUnitToAbbreviation, MapUnitToDefaultAbbreviation, GetDefaultAbbreviation, and GetUnitAbbreviations. Existing UnitKey overloads are refactored to delegate through the new UnitInfo overloads.

The motivation is reasonable — callers who already have a UnitInfo shouldn't need to convert to UnitKey and back, paying an extra registry lookup.

Issues

Typo in XML doc comments — both new MapUnitToAbbreviation and MapUnitToDefaultAbbreviation comments have a double >>:

/// <inheritdoc cref="MapUnitToAbbreviation{TUnitType}(TUnitType,IEnumerable{string})"/>>

The trailing > after the closing /> is invalid XML and should be removed.

No tests — the new UnitInfo-based overloads have no test coverage. Existing tests only exercise the UnitKey paths. Please add at least one test per new overload that calls them directly with a UnitInfo instance (rather than going through the UnitKey delegation path).

Missing blank line — between the closing brace of GetUnitAbbreviations(UnitKey, ...) and the summary comment for GetUnitAbbreviations(UnitInfo, ...), there is no blank line separator, breaking the visual pattern used throughout the file.

Behavior change for UnitKey callers — the existing UnitKey overloads now do an extra Quantities.GetUnitInfo(unitKey) lookup before reaching AddAbbreviation/GetAbbreviationsWithFallbackCulture. This is a minor perf regression for callers using the UnitKey API. The previous PerformAbbreviationMapping went directly to AddAbbreviation. If performance matters here, the UnitKey path should keep calling AddAbbreviation directly (passing the lookup result inline) rather than routing through the new overload.

Summary

The API direction is sound. Please fix the XML doc typos, add tests, and reconsider whether the UnitKey paths should retain their direct lookup path to avoid the extra indirection.

[claude]

PR Review: Unit abbreviations cache unit info overload

Overview: Adds UnitInfo-accepting overloads to MapUnitToAbbreviation, MapUnitToDefaultAbbreviation, GetDefaultAbbreviation, and GetUnitAbbreviations. The existing UnitKey overloads now delegate to the new UnitInfo overloads, removing the internal PerformAbbreviationMapping helper.

---

Positive

• The delegation chain (UnitKey → UnitInfo → AddAbbreviation) is cleaner and avoids a redundant lookup when the caller already has a UnitInfo
• The improved error message in GetDefaultAbbreviation using unitInfo.QuantityInfo.Name + unitInfo.Name is more useful than the old unitKey.UnitEnumType.Name form
• Non-breaking additive change

---

Issues

Double >> typo in <inheritdoc> tags (x2):

/// <inheritdoc cref="MapUnitToAbbreviation{TUnitType}(TUnitType,IEnumerable{string})"/>>

The trailing >> should be />. This will produce malformed XML doc comments.

Missing blank line before GetUnitAbbreviations(UnitInfo, ...):

        public IReadOnlyList<string> GetUnitAbbreviations(UnitKey unitKey, IFormatProvider? formatProvider = null)
        {
            return GetUnitAbbreviations(Quantities.GetUnitInfo(unitKey), formatProvider);
        }
        /// <summary>           ← no blank line here
        /// Retrieves the unit abbreviations...

All other method pairs in this file have a blank line between them.

GetUnitAbbreviations(UnitInfo, ...) doc comment style:
The new overload uses a <summary> block with different phrasing instead of <inheritdoc cref="..."/> like the other overloads. For consistency, prefer:

/// <inheritdoc cref="GetUnitAbbreviations(UnitKey, IFormatProvider)"/>
/// <param name="unitInfo">The info representing the unit.</param>
/// <param name="formatProvider">...</param>

---

Overall: The refactoring is sound and the API addition is useful. Please fix the doc comment typos and the missing blank line before merging.