[mlpack-svn] [MLPACK] #155: order of includes and include protections
MLPACK Trac
trac at coffeetalk-1.cc.gatech.edu
Tue Nov 1 01:08:15 EDT 2011
#155: order of includes and include protections
--------------------------------------------------+-------------------------
Reporter: nslagle | Owner:
Type: wishlist | Status: new
Priority: major | Milestone:
Component: MLPACK | Resolution:
Keywords: includes, templates, style, nitpick | Blocking:
Blocked By: |
--------------------------------------------------+-------------------------
Comment (by rcurtin):
> Another solution adopted by others in the C++ template camp is to use a
different extension for the stuff_impl.hpp file, perhaps stuff.impl (I
can't remember the suffix exactly.)
I've never seen that; the _impl.hpp or _impl.h is fine with me. I don't
see what problem that would solve.
> I'd never seen stuff_impl.h files in addition to stuff.hpp. Additional
code, however frustrating adding four lines to code might be, is well-
worth it in encouraging proper usage.
This is reasonable:
{{{
/** some_file_impl.hpp */
#ifndef SOME_FILE_HPP
#error "Include some_file.hpp, not some_file_impl.hpp."
#endif
#ifndef SOME_FILE_IMPL_HPP
#define SOME_FILE_IMPL_HPP
...
#endif
}}}
But this is superfluous:
{{{
/** some_file_impl.hpp */
#ifndef SOME_FILE_HPP
#ifndef USE_SOME_FILE_HPP
#error "Include some_file.hpp, not some_file_impl.hpp."
#endif
#endif
#ifndef SOME_FILE_IMPL_HPP
#define SOME_FILE_IMPL_HPP
...
#endif
}}}
And, like I stated earlier, this is why:
If someone is dumb enough to define SOME_FILE_HPP but not include
some_file.hpp, then there is no helping them. It's not our problem.
And none of this is about encouraging proper usage. You don't want
someone to include some_file_impl.hpp because there may be declarations in
some_file.hpp which aren't present in some_file_impl.hpp (such as an
inline function) and now everything breaks. In fact there's even a third
solution which is even simpler:
{{{
/** some_file_impl.hpp */
#ifndef SOME_FILE_IMPL_HPP
#define SOME_FILE_IMPL_HPP
// Just in case someone included this directly...
#include "some_file.hpp"
#endif
}}}
It isn't our problem if our users our stupid, but it is our problem if our
library is counterintuitive and poorly designed. In my opinion the
USE_SOME_FILE_HPP method is trying to compensate for idiotic users, which
shouldn't be our problem. I almost like the solution I just proposed (the
just-in-case include) more. Why inundate our users with errors when we
don't need to? Their C++ style is their own problem, and it's definitely
common knowledge that you shouldn't be including the _impl.hpp files.
One more thing:
>> It seems entirely correct for our users to #include <mlpack> and
nothing else.
Well, in a large library like ours that isn't really the right thing to
do. Instead I would expect a user to write `#include
<mlpack/methods/neighbor_search/neighbor_search.h>` and only include what
they need.
> Admittedly, we haven't the time right now to discover exactly which
armadillo dependencies we're using (though likely we aren't using very
many of them)
Modifying our core.h to only include parts of Armadillo is a bad idea. A
user including MLPACK methods is able to assume that Armadillo is
included, and if some methods aren't available because not everything is
included, things break. Yes, we may lose compile time as a result, but
really this is a can of worms we should not open.
--
Ticket URL: <http://trac.research.cc.gatech.edu/fastlab/ticket/155#comment:9>
MLPACK <www.fast-lab.org>
MLPACK is an intuitive, fast, and scalable C++ machine learning library developed by the FASTLAB at Georgia Tech under Dr. Alex Gray.
More information about the mlpack-svn
mailing list