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
962 stars 137 forks source link

Effort to support integral underlying types #125

Open JohelEGP opened 6 years ago

JohelEGP commented 6 years ago

There is interest in having the library support integral underlying types. I believe that the duration class template of std::chrono sets a good example in doing so. Its interface is correct, efficient, easy to use right, and hard to misuse. I propose that we adopt the ways of std::chrono::duration to make this library more generally useful.

To properly support integral underlying types, the following changes need to be implemented. In summary, we start by providing support in the constructors and assignment operators of unit and their dependent convert free function. Then follows the specialization of std::common_type for units to serve as the base of safe conversions between units of the same dimension. Finally, we fix the operations on units to be consistent and behave like the underlying types their arguments represent.

About the last main point, knowledge on the design of std::chrono is very important to correctly apply the framework to stuff std::chrono doesn't have, like the dimensional analysis operations and everything about the non-linear units. I've familiarized myself with it by watching videos about it by its main author, Howard Hinnant, as well as comments from him about std::chrono at StackOverflow, following the standardization process, using the library, reading its specification over and over, trying to generalize the duration interface, and probably in some other minor ways.

I'll try to list the aspects of the duration interface and implementation that will allow us to implement the changes listed above:

nholthaus commented 6 years ago

I'll add that if we get this right, we should also include definitions for additional underlying type units.

JohelEGP commented 6 years ago

we should also include definitions for additional underlying type units.

What do you mean by that? Like how the standard 0s is an integral duration and .0s a floating-point duration?

JohelEGP commented 6 years ago

I think that's about it. The main comment should be complete now.

nholthaus commented 6 years ago

What do you mean by that? Like how the standard 0s is an integral duration and .0s a floating-point duration?

I just mean defining types/alias like meter_t, only with an int underlying type. I'm planning to remove the _t's as part of the 3.0 refactor (although I'd gladly accept user feedback on that), so I'm not exactly sure what I'd want to call them. I think defining a single underlying type for the entire lib is too limiting, especially given your recent work.

I don't think the duration model will quite work for us because I still think the default/most readily accessible underlying type should be floating point.

JohelEGP commented 6 years ago

I just mean defining types/alias like meter_t, only with an int underlying type.

That'd be nice to have.

I'm planning to remove the _t's as part of the 3.0 refactor

That's great. I've read some things about that in previous issues and was planning to mention it to you because I didn't see anything about it for 3.0.0.

(although I'd gladly accept user feedback on that)

Then you'll want to open an issue for visibility.

I don't think the duration model will quite work for us because I still think the default/most readily accessible underlying type should be floating point.

The effort changes nothing in that respect.

nholthaus commented 6 years ago

Are we ready to close this?

JohelEGP commented 6 years ago

Yeah. It seems like you've disabled closing issues from PRs.

nholthaus commented 6 years ago

Right. I don't want them fully closed until they merge into master, but I like to tag them so I know that no more active quirk is required.

On Fri, Jul 13, 2018, 8:21 PM Johel Ernesto Guerrero Peña < notifications@github.com> wrote:

Yeah. It seems like you've disabled closing issues from PRs https://help.github.com/articles/closing-issues-using-keywords/.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nholthaus/units/issues/125#issuecomment-404984241, or mute the thread https://github.com/notifications/unsubscribe-auth/AJ2HH40J6DVnPCvIm4nXec64KPISrYDKks5uGTmlgaJpZM4T8JBD .