PengZheng commented on code in PR #544: URL: https://github.com/apache/celix/pull/544#discussion_r1195863302
########## documents/development/README.md: ########## @@ -0,0 +1,539 @@ +--- +title: Apache Celix Coding Conventions +--- + +<!-- +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> + +# Apache Celix Coding Conventions +Adhering to consistent and meaningful coding conventions is crucial for maintaining readable and maintainable code. +This document outlines the recommended coding conventions for Apache Celix development, including naming conventions, +formatting, comments, control structures, functions and error handling. + +Note that not all existing code adheres to these conventions. +New code should adhere to these conventions and when possible, existing code should be updated to adhere to these +conventions. + +## Naming Conventions + +### C/C++ Variables + +- Use `camelCase` for variable names. +- Use descriptive names for variables. +- Use `celix_` prefix or `celix::` (sub)namespace for global variables. +- Asterisk `*` should be placed on the variable type name. + +### C Structures + +- Use `snake_case` for structure names. +- Add a typedef for the structure. +- Use a `_t` postfix for structure typedef. +- Use `celix_` prefix for structure names. + +### C Functions + +- Use descriptive names for functions. +- Use a `celix_` prefix. +- Use a `_<obj>_` camelCase infix for the object/module name. +- Use a postfix `camelCase` for the function name. +- Asterisk `*` should be placed on the variable type name. +- Use verb as function names when a function has a side effect. +- Use nouns or getter/setter as function names when a function does not have a side effect. +- Use getters/setters naming convention for functions which get/set a value. + +Examples: +- `long celix_bundleContext_installBundle(celix_bundle_context_t* ctx, const char* bundleUrl, bool autoStart)` +- `bool celix_utils_stringEquals(const char* a, const char* b)` +- `celix_status_t celix_utils_createDirectory(const char* path, bool failIfPresent, const char** errorOut)` + +### C Constants + +- Use `SNAKE_CASE` for constant names. +- Use a `CELIX_` prefix for constant names. +- Use `#define` for constants. + +### C Enums + +- Use `snake_case` for enum type names. +- Use a `celix_` prefix for enum type names. +- Use `SNAKE_CASE` for enum value names. +- Use a `CELIX_` prefix for enum value names. +- Add a typedef - with a `_e` postfix - for the enum + +Example: +```c +typedef enum celix_hash_map_key_type { + CELIX_HASH_MAP_STRING_KEY, + CELIX_HASH_MAP_LONG_KEY +} celix_hash_map_key_type_e; +``` + +### Macros + +- Use all caps `SNAKE_CASE` for macro names. +- Use a `CELIX_` prefix for macro names. + +### C files and directories + +- Use `snake_case` for file names. +- Name header files with a `.h` extension and source files with a `.c` extension. +- Organize files in directories according to their purpose. + - Public headers files in a `include`, `api` or `spi` directory. + - Private header files in a `private` and `src` directory. + - Source files in a `src` directory. +- Google test files should be placed in a `gtest` directory with its own `CMakeLists.txt` file and `src` directory. +- Use `celix_` prefix for header file names. +- Use a header guard. +- Use a C++ "extern C" block in headers file to ensure C headers are usable in C++. + +### C Libraries + +- Target names should be `snake_case`. +- There should be `celix::` prefixed aliases for the library. +- C Shared libraries should configure an output name with a `celix_` prefix. + +### C Services + +- Service headers should be made available through a CMake INTERFACE header-only api/spi library (i.e. `celix::shell_api`) +- C service should be C struct, where the first member is the service handle (`void* handle;`) and the rest of the members are + function pointers. +- The first argument of the service functions should be the service handle. +- If memory allocation is needed or another error can occur in a service function, ensure that the return value can + be used to check for errors. This can be done by: + - Returning a `celix_status_t` and if needed using an out parameter. + - Returning a NULL pointer if the function returns a pointer type. + - Returning a boolean value, where `true` indicates success and `false` indicates failure. +- In the same header as the C service struct, there should be defines for the service name and version. +- The service name macro should be all caps `SNAKE_CASE`, prefixed with `CELIX_` and postfixed with `_NAME`. +- The service version macro should be all caps `SNAKE_CASE`, prefixed with `CELIX_` and postfixed with `_VERSION`. +- The value of the service name macro should be the service struct (so without a `_t` postfix +- The value of the service version macro should be the version of the service. + +Example: +```c +//celix_foo.h +#include "celix_errno.h" + +#define CELIX_FOO_NAME "celix_foo" +#define CELIX_FOO_VERSION 1.0.0 + +typedef struct celix_foo { + void* handle; + celix_status_t (*doFoo)(void* handle, char** outMsg); +} celix_foo_t; +``` + +### C Bundles + +- Use `snake_case` for C bundle target names. +- Do _not_ use a `celix_` prefix for C bundle target names. +- Use `celix::` prefixed aliases for C bundle targets. +- Use `snake_case` for C bundle symbolic names. +- Configure at least SYMBOLIC_NAME, NAME, FILENAME, VERSION and GROUP for C bundle targets. +- Use `apache_celix_` prefix for C bundle symbolic names. +- Use `Apache Celix ` prefix for C bundle names. +- Use a `celix_` prefix for C bundle filenames. +- Use a group name starting with `celix/` for C bundle groups. + +Examples: +```cmake +add_celix_bundle(my_bundle + SOURCES src/my_bundle.c + SYMBOLIC_NAME "apache_celix_my_bundle" + NAME "Apache Celix My Bundle" + FILENAME "celix_my_bundle" + VERSION "1.0.0" + GROUP "celix/my_bundle_group" +) +add_library(celix::my_bundle ALIAS my_bundle) +``` + +### C++ Classes + +- Use `CamelCase` (starting with a capital) for class names. +- Use descriptive names for classes. +- Classes should be part of a `celix::` namespace or sub `celix::` namespace. + +### C++ Functions + +- Use `camelCase` for function names. +- If a function is not part of a class/struct, it should be part of a `celix::` namespace or sub `celix::` namespace. +- Asterisk `*` should be placed on the variable type name. +- Use verb as function names when a function has a side effect. +- Use nouns or getter/setter as function names when a function does not have a side effect. +- Use getters/setters naming convention for functions which get/set a value. + + +### C++ Constants + +- Use `SNAKE_CASE` for constants. +- Use constexpr for constants. +- Place constants in a `celix::` namespace or sub `celix::` namespace. + +example: +```c++ +namespace celix { + constexpr long FRAMEWORK_BUNDLE_ID = 0; + constexpr const char * const SERVICE_ID = "service.id"; +} +``` + +### C++ Enums + +- Use `CamelCase` (starting with a capital) for enum types names. +- Use `enum class` instead of `enum`. +- Use `SNAKE_CASE` for enum values without a celix/class prefix. Note that for enum values no prefix is required + because enum class values are scoped. + +Example: +```c++ +namespace celix { + enum class ServiceRegistrationState { + REGISTERING, + REGISTERED, + UNREGISTERING, + UNREGISTERED + }; +} +``` + +### C++ files and directories + +- Use `CamelCase` (starting with a capital) for file names. +- Name header files with a `.h` extension and source files with a `.cc` extension. +- Place header files in a directory based on the namespace (e.g. `celix/Bundle.h`, `celix/dm/Component.h`). +- Organize files in directories according to their purpose. + - Public headers files in a `include`, `api` or `spi` directory. + - Private header files in a `private` and `src` directory. + - Source files in a `src` directory. +- For the Apache Celix framework and utils lib only header-only C++ files are allowed. +- Prefer header-only C++ files for the Apache Celix libraries +- Use a `#pragma once` header guard. + +### C++ Libraries + +- Target name should be `CamelCase` (starting with a capital). +- There should be `celix::` prefixed aliases for the library. +- C++ Libraries should support C++14. + - Exception are `celix::Promises` and `celix::PushStreams` which requires C++17. +- C++ Libraries should prefer to be header-only. +- C++ support for `celix::framework` and `celix::utils` must be header-only. +- Header-only C++ libraries do not need an export header and do not need to configure symbol visibility. +- C++ shared libraries (lib with C++ sources), should configure an output name with a `celix_` prefix. +- C++ shared libraries (lib with C++ sources), should use an export header and configure symbol visibility. + - See the C Libraries section for more information. + +### C++ Services + +- Use `CamelCase` (starting with a capital) for service names. +- Add a 'I' prefix to the service interface name. +- Place service classes in a `celix::` namespace or sub `celix::` namespace. +- Add a `static constexpr const char * const NAME` to the service class, for the service name. +- Add a `static constexpr const char * const VERSION` to the service class, for the service version. + +### C++ Bundles + +- Use `CamelCase` for C++ bundle target names. +- Do _not_ use a `Celix` prefix for C++ bundle target names. +- Use `celix::` prefixed aliases for C++ bundle targets. +- Use `CamelCase` for C++ bundle symbolic names. +- Configure at least SYMBOLIC_NAME, NAME, FILENAME, VERSION and GROUP for C++ bundle targets. +- Use `Apache_Celix_` prefix for C++ bundle symbolic names. +- Use `Apache Celix ` prefix for C++ bundle names. +- Use a `celix_` prefix for C++ bundle filenames. +- Use a group name starting with `celix/` for C++ bundle groups. + +Examples: +```cmake +add_celix_bundle(MyBundle + SOURCES src/MyBundle.cc + SYMBOLIC_NAME "Apache_Celix_MyBundle" + NAME "Apache Celix My Bundle" + FILENAME "celix_MyBundle" + VERSION "1.0.0" + GROUP "celix/MyBundleGroup" +) +add_library(celix::MyBundle ALIAS MyBundle) +``` + +### Unit Tests Naming + +- The test fixture should have a`TestSuite` postfix. +- The source file should be named after the test fixture name and use a `.cc` extension. +- Testcase names should use `CamelCase` (starting with a capital) and have a `Test` postfix. +- When using error injection (one of the `error_injector` libraries) a separate test suite should be used. + - A `ErrorInjectionTestSuite` postfix should be used for the test fixture. + - The error injection setup should be reset on the `TearDown` function or destructor of the test fixture. + +## Comments and Documentation + +- Use Doxygen for code documentation. +- Write comments that explain the purpose of the code, focusing on the "why" rather than the "how". +- Apply doxygen documentation to all public API's. +- Use the javadoc style for doxygen documentation. +- Use `@` instead of `\` for doxygen commands. +- Start with a `@brief` command and a short description. +- For `@param` commands also provide in, out, or in/out information. + +## Formatting and Indentation + +- Use spaces for indentation and use 4 spaces per indentation level. +- Keep line lengths under 120 characters, if possible, to enhance readability. +- Place opening braces on the same line as the control statement or function definition, + and closing braces on a new line aligned with the control statement or function definition. +- Use a single space before and after operators and around assignment statements. +- Add a space after control keywords (`if`, `for`, `while`, etc.) that are followed by a parenthesis. +- Always use braces ({ }) for control structures, even for single-statement blocks, to prevent errors. +- Add a space after control keywords (`else`, `do`, etc) that are followed by a brace. +- Do not add a space after the function name and the opening parenthesis. +- For new files apply clang-format using the project .clang-format file. + - Note that this can be done using a plugin for your IDE or by running `clang-format -i <file>`. + +## Control Structures + +- Use `if`, `else if`, and `else` statements to handle multiple conditions. +- Use `switch` statements for multiple conditions with a default case. +- Use `while` statements for loops that may not execute. +- Use `do`/`while` statements for loops that must execute at least once. +- Use `for` statements for loops with a known number of iterations. +- The use of `goto` is not allowed, except for error handling. +- For C, try to prevent deeply nested control structures and prefer early returns or error handling `goto` statements. + - To prevent deeply nested control structures, the `CELIX_DO_IF` and `CELIX_GOTO_IF_ERR` macros can also be used. + +## Functions and Methods + +- Limit functions to a single responsibility or purpose, following the Single Responsibility Principle (SRP). +- Keep functions short and focused, aiming for a length of fewer than 50 lines. +- Ensure const correctness. +- For C functions with a lot of different parameters, consider using an options struct. + - An options struct combined with a EMPTY_OPTIONS macro can be used to provide default values and a such + options struct can be updated backwards compatible. + - An options struct ensure that a lot of parameters can be configured, but also direct set on creation. +- For C++ functions with a lot of different parameters, consider using a builder pattern. + - A builder pattern can be updated backwards compatible. + - A builder pattern ensure that a lot of parameters can be configured, but also direct set on construction. + +## Error Handling and Logging + +- For C++, throw an exception when an error occurs. +- For C, if memory allocation is needed or another error can occur, ensure that a function returns a value + than can be used to check for errors. This can be done by: + - Returning a `celix_status_t` and if needed using an out parameter. + - Returning a NULL pointer if the function returns a pointer. + - Returning a boolean value, where `true` indicates success and `false` indicates failure. +- Use consistent error handling techniques, such as returning error codes or using designated error handling functions. +- Log errors, warnings, and other important events using the Apache Celix log helper functions or - for libraries - + the `celix_err` (TBD) functionality. +- Always check for errors and log them. +- Ensure error handling is correct, using test suite with error injection. + +## Error Injection + +- Use the Apache Celix error_injector libraries to inject errors in unit tests in a controlled way. +- Create a separate test suite for error injection tests. +- Reset error injection setup on the `TearDown` function or destructor of the test fixture. +- If an - internal or external - function is missing error injection support, add it to the error_injector library. + - Try to create small error injector libraries for specific functionality. Review Comment: Currently, we don't have automatic module dependency deduction within the same package, which I plan to add in a future PR. Note that we do have automatic module dependency deduction between conan packages, which conan gives us for free. Dependency deduction works by manipulating these CMake options in conanfile so that these mocks won't be built if not needed. In that future PR, I also want to make our error injectors installable so that they can be reused by our downstream users. For Celix's own optional module's mock, it can reuse the module's option. For external library's mock, a separate subproject is needed. I found that our error injectors is AI-friendly, and can be generated easily by generative-AI without any IP risks. Large amounts of reusable error injectors will make our users very happy. For now we don't need to mention anything about this in the coding convention. When that future PR is worked out, we can revisit this topic. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org