search

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

`has_num` and `has_den` encourage bad practice #115

Open JohelEGP opened 6 years ago

JohelEGP commented 6 years ago

The problem with has_num and has_den is that they are hardly useful on their own, yet are part of the public interface of the library. It is like the Addable concept which only tests for the presence of a binary operator+, when what we really want is a Number. std::string matches Addable, but not with the expected semantics of a Number.

We already have what we really want, is_ratio_v, so I suggest dropping has_num and has_den from the public interface for v3.0.0. I'm already working on some patches which include these changes, but I'm opening an issue anyways. The patch is a simplification of is_ratio that depends on the library relying only on std::ratio and not a Ratio implementation anymore which I found digging through the commits.

Note that the patches I'm working on are pretty simple, because I know you're working on refactoring the code for v3.0.0. For more complex stuff I'll be opening issues instead.

nholthaus commented 6 years ago

@johelegp agree. Appreciate both the issues and the patches. Thanks for pushing me and the library to take the quality to the next level.

Really wish we had an std::is_ratio_v though...