v5: Wishlist for breaking changes

[angularsen]

Although there are no immediate plans to do a vNext, this issue serves as a wishlist/reminder of what to consider changing in case we need to bump the major version in the future.

To clarify

These are some things we should figure out before finalizing version 4.

A common denominator for design choices is binary size.
The library was nearing the 1MB mark and the majority of that is the large number of methods and properties for all our 700+ units. In particular number extension methods add 8 methods per unit (!). With some clever hacks we brought this back to 600kB without any breaking changes, but we want to keep binary size in mind for any new changes we introduce.

TODO Clean up and organize this brain dump:

• Numeric type for value representation: Keep double? Switch to decimal? Offer both? Separate nugets? What about float?
• Base types: To make it easier working with quantities generically, accessing value and unit, get units and their abbreviations, parsing and ToString() without know the quantity type in advance.
• struct vs class: Weigh all the pros and cons, performance (stack vs heap), inheritance vs interface
  • class: Can support all number types (float, double, decimal) without increasing binary size N times (discussion)
  • struct: More suitable for performance and memory constrained applications (what is the impact in practice?), avoids pressure on garbage collector, quantities match the semantics of being a value type

List of breaking changes to make

Removing things

• 7a455f0: Remove default Temperature arithmetic, it is not correct (made this breaking change already, due to existing behavior not being correct).
• Remove UnitClass type, replaced by QuantityType
• Remove nullable From factory methods causing N additional methods for N units, see comment
• Remove == != and Equals(object) and Equals<T>(T) since they don't take a max error argument and is prone to floating point rounding issues, instead users should use Equals() methods that take a max error argument
• Remove code marked as [Obsolete], such as Volume.Teaspoon unit that was too ambiguous
• Remove static methods on UnitSystem, should use UnitSystem.Default instead
• Remove unnecessary overloads on number types (8 overloads for each of the 700+ units) Proposal: Reducing size of library (WIP) #372

Changing behavior

• Throw exception on NaN values in unit class constructor methods (Support NaN #176)
• Throw if unit was not specified when constructing quantities Preserve value and unit #389 (comment)
• Stricter parsing (see "Length.TryParse" parses invalid values #343 and v5: Wishlist for breaking changes #180 (comment))

Renaming

• Acceleration units missing plural s in Meters (fixed in Plural fix for accelerations #434)
• Correct SingularName for some Flow unit definitions (see Add more Flow units #360)

Fixing

• Search for any TODO comments in the code to address, remove comment and either fix or create issue

[angularsen]

Ping @JKSnd @eriove @ferittuncer to subscribe to updates to this thread.

Question: Should we change this

TemperatureDelta.FromDegreesCelsiusDelta(50);

to this

TemperatureDelta.FromDegreesCelsius(50);

Delta is already covered by the quantity, so it feels redundant. Just noticed this when reviewing https://github.com/angularsen/UnitsNet/pull/324/files#diff-1067499ac8bb37a45676058d184d0547R55

[eriove]

Yes, that sounds like a good change. We could add the new function and mark the old one as obsolete immediately. That would make the migration easier

[gojanpaolo]

Please add this to wishlist: Correct SingularName for some Flow unit definitions.

see #360

[bplubell]

I propose making parsing stricter:

• Fail when parsing with multiple sets of units.
  • Passing example: Length.Parse("2.3 m")
  • Failing example: Length.Parse("2.3 m and 200mm")
• Special handling for FeetInches.
  • Passing example: Length.Parse("1ft 5.5in") and Length.Parse("1\"5.5'")
  • Failing example: Length.Parse("1ft and 4in")

This would be more in consistent with framework parsing methods (like double.Parse), which are not very lenient. The burden lies with the consumer to provide data that is not ambiguous.

See #343 for additional context.
Related PRs from originally allowing multiple units:
#64
#81
#254

[angularsen]

@bplubell How about we actually have a separate parsing method for the feet-inch special case? Something like Length.ParseFeetInches(string) ? I think maybe it is reasonable to make it explicit that we are expecting such input, as opposed to having this special case as part of Length.Parse().
Or maybe this just complicates things from the consuming side of things.. I'm not sure.

[gojanpaolo]

Remove inconsistent abbreviation for cubic feet cf/hr in Flow (See #361 (review))

[bplubell]

@angularsen Re: ParseFeetInches. That might work, though I am wondering about passing in a value that a user has input (like from a form). Should UnitsNet be responsible for parsing all valid input (even special cases), or should the code calling into UnitsNet be responsible for knowing the special cases and calling the right parsing method?
I lean toward the first one, though if we do that we would very likely want an internal method like Length.ParseFeetInches that is called after we determine that it is a special case. Something along the lines of:

Public bool TryParse(string value, out Length length)
{
    if (!QuantityParser.TryParseExact(value, out length))
    {
        if (!Length.TryParseFeetInches(value, out length))
        {
            return false;
        }
        // else if over additional special cases if necessary
    }
    return true;
}

[angularsen]

@bplubell Good example. I favor keeping it simple from the user's stand point, so that Length.Parse() and Length.TryParse() internally tries a series of special cases in sequence, as you propose here, so that it "just works" as before with strings like 1' 4" and 1 ft 4 in, but the same does not work for other quantities unless explicitly added as special cases. I do want to be a lot stricter on the allowed variants of these foot-inch inputs so we only allow well-formed inputs that are actually widely used.

[angularsen]

I have created a new branch v4 that we will target the pull requests in this wishlist, and any other changes we want to include in only v4 and not in 3.x versions.

It will be a fairly big undertaking to complete all this, but I propose we start on this before the list grows unmanageable and the jump from 3.x to 4.x becomes too great from the consuming side.

[tmilnthorp]

Acceleration units missing plural s in Meters is fixed in #434

[angularsen]

Updated the checklist

[tmilnthorp]

I guess we should have bumped the major number for that eh :)

[angularsen]

Right, my bad, I thought we obsoleted the old units. We should probably add the old ones back and mark them obsolete.

[angularsen]

vNext is long overdue now, I muted thousands of build warnings about obsolete code earlier because they just spammed the build log and we don't take any dependencies I expect to be obsoleted anytime soon.

[angularsen]

Added to list:

• Remove nullable From factory methods causing N additional methods for N units, see comment

And organized list into category of changes.

[angularsen]

Closing this since most of the discussions above are covered in v4 #487.
Creating new discussions for the remaining topics, such as changing from struct to class.