NWChemEx / .github

GitHub Settings for the NWChemEx Organization
https://nwchemex.github.io/.github/
Apache License 2.0
1 stars 2 forks source link

Other C++ Conventions #16

Open ryanmrichard opened 3 years ago

ryanmrichard commented 3 years ago

We have a list of C++ coding conventions here, but there's other aspects of the C++ source code design which should be standardized. I'm starting this issue to keep track of such things.

This should be compared with the C++ Core Guidelines for consistency.

ryanmrichard commented 2 years ago

FWIW I just reacquainted myself with the C++ Core Guidelines. It's filled with great suggestions that everyone writing modern C++ should read; however, it does little to help us with most of the conventions listed here, aside for suggesting we mirror the standard libraries as closely as possible.

To start the ball rolling here's my takes:

use of _t and _type suffixes for typedefs

  • The standard library tends to use _type for typedefs which are members of classes (e.g. size_type, value_type) and _t for typedefs outside classes (e.g. tuple_element_t)
  • I suggest we follow suit decide whether to use _ as prefix or suffix on protected/private members
  • This should be a suffix; prefixes are reserved for the standard libraries use m_ prefix for members
  • Here's the common scenario:
    class Example{
    public:
    void set_x(x_type x) { m_x_ = x; }
    private:
    x_type m_x_;
    };

    Prefixing the member of the class with m_ helps disambiguate it from the input value to the set_x function.

  • While not phrased as a question, I guess the question is "what convention to use?". A quick glance at the code for std::vector suggests they actually use M_ and not m_. IMHO m_ is more consistent with our other conventions and thus my recommendation. insist on out-of-line definitions for functions that are longer than one line
  • This is a cleanliness thing. For better or worse we usually end up having to look at class declarations for one reason or another. Having member definitions intertwined makes it harder to peruse the declaration.
  • If you put them after the class they still get inlined.
    put out-of-line definitions to be inline'd in .ipp files
  • Mainly a cleanliness issue. Having a lot of inline definitions following the class can be overwhelming.
  • Having a separate .ipp file makes it easier to compare the declarations to the definitions a la typical .hpp vs .cpp comparisons (i.e., you can use two windows/editors)
  • I'd vote we do this when we have a number of such functions say >2. Putting a couple inline definitions in an .ipp file seems like overkill. names of polymorphic copy and comparisons
  • Typical convention is to make the copy ctor, and the comparison operators (e.g. operator==) only consider the state in the current class (and its base classes) how to reference types from dependencies
  • For example how does Chemist use the pluginplay::Hasher type?
  • Using pluginplay::Hasher directly means we have a lot of places to change in Chemist if PluginPlay changes the type. AFAIK the recommendation is to have Chemist alias the pluginplay::Hasher type and to use the alias throughout Chemist.
  • The question is how do we want to standardize this? Put external types used throughout Chemist in include/chemist/types.hpp?
  • IMHO classes should continue to define their own member types, and use those member types throughout the declaration/definitions. This allows the class to define its own source of truth (which can simply alias the library source of truth) in case it needs to act independently of the rest of the library.
    directory structures?
  • I think we decided on the GNU structure at the top-level (public API in include, private implementations in src). This is more concerned with how its organized under those directories.
  • My vote is for mirroring the namespace structure. So the .hpp file for a class pluginplay::any::detail_::AnyBase would live in include/pluginplay/any/detail_/. when to use nested namespaces
  • I use nested namespaces in two main scenarios: specifying something is an implementation detail (the detail_ namespace) and to factor out parts of the class name (e.g. instead of MarkdownPrinter, SphinxPrinter, JSONPrinter do printer::Markdown, printer::Sphinx, and printer::JSON).
  • Other than that I think it's a case-by-case basis. IMHO when you get a more or less self-contained series of classes that all work very intimately together it makes sense to put them in a namespace (think of it as a sublibrary) where to put traits
  • My vote is that type traits for a given namespace go in namespace/type_traits/type_traits.hpp.
  • I vote for a folder because that makes it easier to have type_traits.hpp be a convenience header, and have groups of related traits reside in their own header files. when to start new files
  • this was touched on in a couple places, but my suggestions:
  • one class/struct per file (exception simple structs used in template metaprogramming)
  • A well documented class declaration tends to be quite large
  • free functions related to a class/struct can go in the same file
  • related free function declarations can go in one file
  • the associated free function definitions should go in different files unless the definitions are all short
  • related type traits go in one file
  • my general guideline is to shoot for file lengths that are under 300 or so lines
  • IMHO it's much easier to find things manually with fine-grained files

FWIW I know trying to standardize some of these things may seem pedantic, but uniformity and consistency make coding, and maintaining that code, much easier for both established and new coders.