nholthaus / units

a compile-time, header-only, dimensional analysis and unit conversion library built on c++14 with no dependencies.
http://nholthaus.github.io/units/
MIT License
934 stars 134 forks source link

v2 -> v3 porting #313

Open Guillaume227 opened 1 year ago

Guillaume227 commented 1 year ago

I have been attempting to convert a codebase using version 2 to the 3.x branch and here is some feedback and questions. I embarked on this upgrade because I grew tired of the inconsistent to_string vs stream behavior for units like volt_t. My initial, naive, expectation was a drop-in replacement with no change needed in the rest of the code base.

Here is what happened in the hope that's useful for people going through an update. I am planning to send a few PRs as follow-up.

Most of the issues stemmed from using a float underlying type - all of the unit testing is done for double.

  1. v2 Types are templates in v3 I used to do:
    
    #define UNIT_LIB_DEFAULT_TYPE float 
    using meter_t = units::length::meter_t;
In 3.x I do:
`using meter_t = units::length::meter_t<float>;
`

2. **Dealing with literal operators and underlying type**
In 3..x it a bit less nicely packaged when it comes to literal operators since I am dealing with floats.
I have to redefine them outside of core.h with a user-land macro since I do not want `double`:

constexpr namespaceName::nameSingular##t operator""##abbreviation(long double d) noexcept \ { \ return namespaceName::nameSingular##_t(static_cast(d)); \ }

It's not too bad though, I just had to reintroduce `UNIT_HAS_LITERAL_SUPPORT` in 3.x.

3. **units::math**
- It's easy enough to figure out that the math functions are directly available in the units namespace and that units::math is gone.
- In a more subtle way, I started to get compilation errors with units::sqrt of the no-template-overload-found variety.
I tracked it down to a 0.5 literal that was causing a mixture of double and floats in the call to `sqrtNewtonRaphson`. Once found, it was straightforward to replace that with T{0.5}.

4. **Ternary operator and arithmetic operators+,- return type**
I hit an issue where the ternary operator no longer compiles:

degree_t val1 = 10_deg; degree_t val2 = 90_deg; degree_t new_val = some_bool ? val1 - val2 : val2;


After sifting through the template compilation errors, it's down to:
`std::common_type_t<degree_t, degree_t>` differing from `degree_t`, as it's using a dummy 1 conversion factor.
That can be solved through another `std::common_type_t` specialization.
Guillaume227 commented 1 year ago

Wait, it seems I was missing some significant updates to v3.x, for example the _t suffix is gone entirely now. I need to review and update what I said above.

Guillaume227 commented 1 year ago

Ok after a bit of renaming I managed to make almost everything work on 3.x - I updated all the PRs above, they should be good for review/feedback. The one thing that's not working for me in 3.x as it used to on 2.3 is to_string for on-the-fly units, e.g. : to_string(1_A * 1_A); It used to print: 1 A^2 but now I am getting a compilation error. Not a bit deal for me at the moment, I might look into that at a later stage unless someone has a better understand of what's going on here.

I also opened #318 and #317, they are self-explanatory

Overall things feel much cleaner in 3.x, Thanks. Straightening up the ADL story makes a big difference.

Regarding testing, my use case makes use of float underlying_type which makes me notice how current tests cover double underlying_type. I am not sure how to extend that without a lot of code duplication. Something like templating the test cases on underlying type?