Skip to content

v5: Wishlist for breaking changes #180

Closed
@angularsen

Description

@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

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

Activity

angularsen

angularsen commented on Nov 26, 2017

@angularsen
OwnerAuthor

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

eriove commented on Nov 27, 2017

@eriove
Contributor

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

gojanpaolo commented on Jan 9, 2018

@gojanpaolo
Contributor

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

see #360

bplubell

bplubell commented on Jan 11, 2018

@bplubell
Contributor

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

angularsen commented on Jan 13, 2018

@angularsen
OwnerAuthor

@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

gojanpaolo commented on Jan 14, 2018

@gojanpaolo
Contributor

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

12 remaining items

angularsen

angularsen commented on Oct 15, 2018

@angularsen
OwnerAuthor

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.

added a commit that references this issue on Dec 16, 2018
added a commit that references this issue on Nov 29, 2022
9574d47
changed the title [-]vNext: Wishlist for breaking changes[/-] [+]v5: Wishlist for breaking changes[/+] on Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @angularsen@tmilnthorp@eriove@bplubell@gojanpaolo

      Issue actions

        v5: Wishlist for breaking changes · Issue #180 · angularsen/UnitsNet