mpusz / mp-units

The quantities and units library for C++
https://mpusz.github.io/mp-units/
MIT License
1.07k stars 85 forks source link

Bikesheding of `quantity_point::relative()` #479

Closed mpusz closed 1 year ago

mpusz commented 1 year ago

quantity_point::relative() is considered "unsafe" as discussed in https://github.com/mpusz/mp-units/issues/414#issuecomment-1663864071 but also really needed as mentioned in https://github.com/mpusz/mp-units/issues/414#issuecomment-1665560577.

We can discourage its use by making the name longer 😉

One of the proposals was to name it relative_to_origin(), but after a longer thought, I am not sure if the "relative" is the correct name here. We are used to it because it is n mp-units "forever", but it might not be a correct term in the domain.

Even our documentation says explicitly that the affine space is about points and vectors where we model a vector with the quantity class template. So another candidate could be vector_from_origin. This, however, might be also unfortunate as we also support vector and tensor quantity characters as documented here: https://mpusz.github.io/mp-units/2.0/users_guide/framework_basics/character_of_a_quantity and those names would collide and make it hard to understand what kind of "vector" we talk in each context.

So maybe it should be just called quantity_from_origin()?

Wikipedia article https://en.wikipedia.org/wiki/Affine_space also mentions "displacement vectors, also called translation vectors or simply translations" so maybe a translation_from_origin() is a good name here.

Any thoughts?

JohelEGP commented 1 year ago

quantity_from_origin is a nice generalization from std::chrono::time_point::time_since_epoch.

mpusz commented 1 year ago

Yes, and it nicely complements quantity_point::point_from(PointOrigin).

burnpanck commented 1 year ago

I think this should be resolved in analogy to #476: Instead of the unsafe .relative(), we could have a save.relative_to(PointOrigin) that returns an rvalue, as-well as a .stored_quantity_relative_to(PointOrigin) (name of course still to be bike-shedded) that either returns an appropriate lvalue reference or fails to compile.

JohelEGP commented 1 year ago

So this is really #477, but for quantity_point's stored quantity.

chiphogg commented 1 year ago

Some thoughts so far:

JohelEGP commented 1 year ago

"Offset" is another alternative to the overloaded "vector". Or maybe it doesn't matter, since they're at different levels of abstraction. It could be that giving them different names is worse than calling them by what they are.