Closed jgvictores closed 7 years ago
PS: Edited grammar from/to error above.
It was done on purpose: I moved every single header inclusion directive on that repo, from the .h/.hpp files to the .cpp it's actually used in (and I followed the same pattern on roboticslab-uc3m/gait and asrob-uc3m/robotDevastation).
PS: I do understand I may be in contradiction with myself, as I usually prefer to keep things as close as possible to the scope where they are used.
Not just that. Keep in mind that you are increasing compilation time for every translation unit within the final executable/library target (in fact, we tend to use the single .hpp/multiple .cpp directory structure, thus every source file includes that header). Refer to this SO answer.
A really compelling reason not to bloat your main header file with dependencies is... the dependencies themselves. It doesn't matter on a MODULE
target like KdlSolver or any other YARP plugin we create nowadays, but you must not include additional dependencies in your exported header file in the case of library targets. We had a talk some weeks ago about including color-debug at ICartesianSolver.hpp
, leading to the conclusion that any program that uses this header must be able to locate ColorDebug.hpp as well. In case you want to separate the interface from the implementation in a shared library, then keeping your #include's close to the scope they are used in is the way to go.
See Google C++ Style Guide:
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
It's not stated explicitly, but you may infer a similar conclusion from the examples: why were all those additional #include's added to the .cc file instead of being used inside "foo/server/fooserver.h"
?
@PeterBowman Thanks a lot for all of these reasons and references in favor of the current implementation!
I'll add a link to the Q/A repository too.
I just want to leave this here to illustrate a case where minimizing #include
s is hazardous (which is vaguely related to this issue, but still...). Compare these two CI weekly cron builds of the tools repository on consecutive commits, no changes in between:
One cannot depend on headers that may or may not include another header, which in turn happens to be a must for your code to compile, and that's why I tend to prefer explicitness. This was the fix: roboticslab-uc3m/tools@da3ee49.
That's interesting. :+1:
Relevant SO question: self-sufficient (self-contained) headers.
I'd prefer to minimize the number of #include directives in .cpp files. An anti-example would be https://github.com/roboticslab-uc3m/kinematics-dynamics/blob/develop/libraries/TeoYarp/KdlSolver/ICartesianSolverImpl.cpp#L5-L10 @PeterBowman Would you mind if I moved these directives from ICartesianSolverImpl.cpp to the KdlSolver.hpp header, or do you have special preferences for this approach?
PS: I do understand I may be in contradiction with myself, as I usually prefer to keep things as close as possible to the scope where they are used.