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

Minimize over-inclusion to reduce accidental complexity in header files #715

Open
pnoltes opened this issue Jan 4, 2024 · 1 comment
Open
Labels
build/environment Categorizes an issue or PR relevant to the build environment. kind/wish Categorizes issue or PR as a wish.

Comments

@pnoltes
Copy link
Contributor

pnoltes commented Jan 4, 2024

Intro

If (public) header files include too much upstream header files, this can increase accidental complexity (accidental complexity arises when a software system becomes unnecessarily complicated due to factors that are not inherent to the problem being solved, but are rather a byproduct of the chosen tools, technologies, or approaches).

Several reasons over-inclusion add accidental complexity:

  • Over-inclusion increases the compilation time as the compiler needs to process a larger set of dependencies
  • Over-inclusion leads to a bloated and less maintainable codebase, where changes in one part of the system can have unforeseen ripple effects.
  • Over-inclusion of headers can also inadvertently expose internal details to downstream users.
  • Over-inclusion can lead to unintentionally backward incompatibility when header files are updated.

Minimize symbols

An alternative to including a header file which contain many unneeded symbols is to use forward declaration or only include header files with a minimal number of symbols, forward declaration and/or opaque typedefs.

This will prevent adding to much unneeded symbols during a compilation of a source file and prevent downstream users to accidental/unintellionally get symbols they did not specifically include.

Note that creating header files with minimal symbols/forward-declaration/opaque-typedefs, can only be done for Apache Celix own header files.

Tasks

Update the coding guidelines

Update the coding guidelines so that functional coherent C header files are split up in multiple header files using something like the following scheme:

  • The public api for a C functionality can can be found in a header named: celix_<functionality_snake_name>.h
  • The opaque typedefs (forward declaration) can be found in a header named: celix_<functionality_snake_name >_type.h. This header ideally should only contain opaque typedefs, but can also contain concrete struct definition if needed (e.g. celix_properties_entry_t or celix_properties_value_type_e, etc)
  • If needed all private functions or structs (only used inside the library which provided the public api) can be found in a header named: celix_<functionality_snake_name >_private.h. These header files should be in the src dir.
  • If needed all internal functions or structs (only used inside the Apache Celix project, but not meant as public API), can be found in a header named: celix_<functionality_snake_name >_internal.h. These header files should be in a separate include_internal dir and should only be added to a CMake target for the BUILD scope (not for install).

And functional coherent C++ header files are split up in multiple header files using something like the following scheme:

  • The public api: celix/Functionality.h
  • The forward declaration and minimal symbols: celix/FunctionalityType.h
  • If needed a private header (in src dir): celix/FunctionalityPrivate.h
  • if needed a internal header (in include_internal): celix/FunctionalityInternal.h

Update framework and libs

Update at least the framework and libs to following the aforementioned updated header coding style. The bundles and bundle api/spi libs can be updated using a the last Boy Scout rule.

Optional add build tooling to check include behaviour.

Maybe it is possible to use something like include what you use tool to check include behaviour.

@pnoltes pnoltes added build/environment Categorizes an issue or PR relevant to the build environment. kind/wish Categorizes issue or PR as a wish. labels Jan 4, 2024
@pnoltes
Copy link
Contributor Author

pnoltes commented Jan 4, 2024

note feedback is welcome

pnoltes added a commit that referenced this issue Feb 11, 2024
Also fix indentation in manifest_parser.c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build/environment Categorizes an issue or PR relevant to the build environment. kind/wish Categorizes issue or PR as a wish.
Projects
None yet
Development

No branches or pull requests

1 participant