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
952 stars 135 forks source link

No more nested namespaces in `units` #160

Open JohelEGP opened 6 years ago

JohelEGP commented 6 years ago

Following https://wg21.link/p0816 and common design guidelines, I suggest the following.

Mark the namespaces in which the units are defined as inline. The README suggests the namespaces are used to avoid name clashing, for example, mass::pounds and force::pounds. By marking the namespaces as inline, these rare name clashes are avoided, as attempting to use something like units::pounds becomes a compile-time error. We also benefit from using most units by simply prepending them with units:: and avoiding the pitfalls used to circumvent nested namespace designs. These names might be noise in their context of use, like the length in units::length::meters, or plain overly long, like units::magnetic_field_strength::tesla, to pick one. The user is still free to use them if they so decide.

nholthaus commented 6 years ago

in addition we should split the different unit types into constituent header files. I've resisted this in the past because have a single-header library is kind of nice, but the compile-time benefits would be enormous, since (in my experience), unit types can rarely if ever be forward declared. It's pretty much the only idea we have at present to improve compile times, which is the main barrier to user adoption (along with better docs perhaps).

JohelEGP commented 6 years ago

There's an ambiguity between the min function and the min abbreviation when inlining the units::time namespace. That can't be solved without choosing not to define that min alias.

nholthaus commented 6 years ago

Let's replace it with 'mins' then.

On Wednesday, July 25, 2018, Johel Ernesto Guerrero Peña < notifications@github.com> wrote:

There's an ambiguity between the min function and the min abbreviation when inlining the units::time namespace. That can't be solved without choosing not to define that min alias.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.< https://ci6.googleusercontent.com/proxy/fUrIuPWk_GzctYkSFhSSqD9auxEBgQV05LVMh2TSq1R3ba1qnsWLp_9GXJgKEXGK5NjAJHMBgvJfwhGUs2qlmHPvZcYzkWY9WRwg9Ij2t7lKbzcSiv4alQHcpxnn0-oNFhNyxlW2j29oqOcHJIB5sB_tUEfuqg=s0-d-e1-ft#https://github.com/notifications/beacon/AJ2HH4DicjlJky0EDPuoB17VoxI46y_eks5uKKQUgaJpZM4VVAN0.gif>

JohelEGP commented 6 years ago

Doing so there will also make it so that the UDL uses mins rather than the SI min. I'd prefer to keep the meaning of 1_min and only change the tag to mins.

JohelEGP commented 3 years ago

Note that this can break user code as new units are added.