Decide on convention for `detail` directories

[brianhou]

There's a detail directory in src/planner/vectorfield, although most of our detail directories are in include somewhere. I feel like we should have some consistent rules that are written down somewhere.

@mkoval, @jslee02 mentioned that you had some thoughts about this?

[jslee02]

IIRC, @mkoval meant to put detail headers to src directory unless it includes templates in it. I believe the motivation behind this is to completely hide details.

I personally prefer having all the headers including detail headers in include directory, where those detail definitions should be under detail namespace. But I'm open to following @mkoval's convention. Consistency is almost always more valuable. :sunglasses:

Yeah, once we all agree on one convention, let's write down the convention to the STYLE.md and follow it.

[psigen]

I suspect that the reasoning behind this is that putting all the detail headers into include means that they typically get exported as part of the public headers packaged for distribution.

This is obviously not exactly desirable, as it unintentionally clutters up the public headers with declarations that should not be exposed to end users.

On the other hand, the extra headers shouldn't actually get compiled or used anywhere, and being in the detail subfolder makes it fairly clear that end-users should not normally be using them. So maybe it is worth it for the simplicity. :man_shrugging:

[mkoval]

I personally prefer having all the headers including detail headers in include directory, where those detail definitions should be under detail namespace. But I'm open to following @mkoval's convention. Consistency is almost always more valuable. 😎

This is not . It is not possible to define (note: define != declare) a variable in a header because it may be included in multiple translation units. That being said, it's rare (and probably a bad idea!) to define global variables in adetail directory.

I suspect that the reasoning behind this is that putting all the detail headers into include means that they typically get exported as part of the public headers packaged for distribution.

This is the bigger issue. In my opinion, we should avoid putting implementation details in the include directory except when absolutely required. The include detail directories only exist because there is no other place to define the implementation details of templates.

---

In my mind, the key question is: Where should should we put large blocks of helper code that are not part of Aikido's public API?

A great example of this the VectorFieldIntegrator helper class used inside the followVectorField() function. This is entirely an implementation detail, so it doesn't really belong in include. However, it is big enough that it shouldn't be shoved inline in an anonymous namespace in VectorFieldPlanner.cpp.

Since it can't live in anonymous namespace, it must live in some namespace. The detail namespace makes sense to me, since that is the convention we use in header files. Since it lives in the detail namespace, it should also live in a detail directory. 😄

---

Here is the convention I propose:

The detail Namespace and Subdirectory

It is sometimes necessary or desirable to define helper functions or types while implementing a function. If this is purely an implementation detail, these functions or types should not be exposed in Aikido's public API.

In order of preference, Aikido prefers to place this type of helper code in the following locations:

1. If it is a small amount of code that is only used in one .cpp file, put the helper code in an anonymous namespace at the top of the file.
2. If it is a large amount of code code or is needed in multiple .cpp files, place the code in the detail namespace and split it into one or more files in a detail subdirectory of the src tree.
   • Code in detail directories in the src tree should follow the same convention as files implemented elsewhere, i.e. the declaration (in header files) should be split from its implementation (in source files).
   • Unlike the rest of Aikido, detail directories in the src true can contain both header files and source files. These header files are an implementation detail and, thus, should not be placed in the include directory.
3. If the code is used in a template class or function that is part of Aikido's public API, place the implementation in the detail namespace and split it into one or more files inn the detail subdirectory of the include tree.
   • This should be considered a last resort. If a template is only used on a small set of potential parameters (e.g. float and double), consider moving the implementation into the src tree and explicitly instantiating the template on those parameters.

Thoughts? It's a bit of a mouthful. 😕

[jslee02]

I agree on @mkoval's propose for helper functions assuming we don't expose the helper functions as AIKIDO's public API. For those API, we could completely remove from include directory.

One exceptional category of API I'm thinking of is this: API that should be exposed to the public API because it's used in other header files, but it's rarely used in general use cases. In that case, we might want to move the rarely used API under detail subdirectory/namespace so that we don't expose too many details.