From 445fce0c3fa5385c5e712ab027a98670ca64e7d1 Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Thu, 19 Jan 2023 10:29:44 -0800 Subject: Add a new common error While this is more of a question of style, we have enough problems with our lambda usage that it is worth having some documentation we can point people to in review. Signed-off-by: Ed Tanous Change-Id: I2a3f819693dfd6a9a46f92037574c5d28c3121e5 --- COMMON_ERRORS.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'COMMON_ERRORS.md') diff --git a/COMMON_ERRORS.md b/COMMON_ERRORS.md index 70c09e3476..e114e64029 100644 --- a/COMMON_ERRORS.md +++ b/COMMON_ERRORS.md @@ -328,3 +328,42 @@ binaries. Note, another form of this error involves calling nlohmann::json::reset(), to clear an object that's already been filled in. This has the potential to clear correct data that was already filled in from other sources. + +## 14. Very long lambda callbacks + +```cpp +dbus::utility::getSubTree("/", interfaces, + [asyncResp](boost::system::error_code& ec, + MapperGetSubTreeResult& res){ + + }) +``` + +Inline lambdas, while useful in some contexts, are difficult to read, and have +inconsistent formatting with tools like clang-format, which causes significant +problems in review, and in applying patchsets that might have minor conflicts. +In addition, because they are declared in a function scope, they are difficult +to unit test, and produce log messages that are difficult to read given their +unnamed nature. + +Prefer to either use std::bind_front, and a normal method to handle the return, +or a lambda that is less than 10 lines of code to handle an error inline. In +cases where std::bind_front cannot be used, such as in +sdbusplus::asio::connection::async_method_call, keep the lambda length less than +10 lines, and call the appropriate function for handling non-trivial transforms. + +```cpp +void afterGetSubTree(std::shared_ptr& asyncResp, + boost::system::error_code& ec, + MapperGetSubTreeResult& res){ + +} + +dbus::utility::getSubTree("/xyz/openbmc_project/inventory", interfaces, + std::bind_front(afterGetSubTree, asyncResp)); +``` + +See also the +[Cpp Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f11-use-an-unnamed-lambda-if-you-need-a-simple-function-object-in-one-place-only) +for generalized guidelines on when lambdas are appropriate. The above +recommendation is aligned with the C++ Core Guidelines. -- cgit v1.2.3