Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Splitting unit definitions into different headers #164

Open
JohelEGP opened this issue Jul 21, 2018 · 6 comments
Open

Splitting unit definitions into different headers #164

JohelEGP opened this issue Jul 21, 2018 · 6 comments
Milestone

Comments

@JohelEGP
Copy link
Contributor

Related issues/comments:

Regarding #48 (comment)

deprecated by the define/cmake options to opt-in to specific namespaces

By splitting and not relying on those macros, there's less chances of clash between depending projects using units.

@nholthaus
Copy link
Owner

We should remove those macros when we split. opt-out was better than nothing, but opt-in will be a lot easier and more intuitive on the user.

Given how much work has been done to make units easy to install, and its availability in package managers, I don't see any downsides.

@nholthaus nholthaus added this to the v3.0.0 milestone Jul 25, 2018
@JohelEGP
Copy link
Contributor Author

How granular should the headers be? Per unit would result in a lot of headers, probably to the point of being impractical. Per dimension is another option, like how they are currently separated (as indicated by the is_..._unit type trait.) There could additionally be headers of logical groups, like one including units of length, area, and volume.

How about the directory layout? I'd expect an all-including header and the granular headers to reside within the units directory. I prefer units.h as the all-including header, but I've also some like units/units.h and units/all.h.

We'd need a units/fwd.h header so as to be able to implement the "strengthening" of units without ODR issues. And probably a units/core.h or similar header, that has everything but unit definitions, except for the dimensionless units, the other ones required by the mathematical functions, and the constants.

@JohelEGP
Copy link
Contributor Author

Actually, unless you're thinking of being conservative on the mathematical functions and constants we have, they should also be split as to prevent someday having too many of them in the core header.

@nholthaus
Copy link
Owner

My original plan was to split by dimension, and leave dimensionless and core lib stuff (and pi) in units.h.

I like having a units.h as an alll-inclusive in the base directory to maintain some semblance of backwards compatibility,

Constants become funny. I'd make a units/constants.h and include whatever is required. My assumption is people who are like, "why is Mu0 not defined!" won't really care about optimization. We'll see.

Math functions I'd leave in units/h, except for angle-only functions which I'd put in angle.h.

We'd need a units/fwd.h header so as to be able to implement the "strengthening" of units without ODR issues.

Can you elaborate a bit?

That said, I have the utmost respect for your opinion, so let me know if you have other ideas.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Jul 26, 2018

That said, I have the utmost respect for your opinion, so let me know if you have other ideas.

I appreciate it!

Can you elaborate a bit?

See #167.

Let's see what we have...

  • units.h, #includes everything below and the constants.
    • units/core.h or similar, including everything but UNIT_ADDs, the math functions that use angle, and the constants, including forward declarations as necessary.
    • units/dimension.h, each of which #include <units/core.h>, the above header of forward declarations, and their respective UNIT_ADDs.
      • units/angle.h also contains the math functions that use angle.

@nholthaus
Copy link
Owner

sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants