Skip to content

add a comparison of classic and modern polymorphism #10

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

Merged
merged 42 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
a0bea38
add first concepts example
daixtrose Oct 9, 2024
0f46bcb
remove clutter from CMake config
daixtrose Oct 10, 2024
b2c3038
refined demo of interface vs. concept
daixtrose Oct 15, 2024
65ebad8
add first unit test
daixtrose Oct 15, 2024
64002ae
rename example_5 to polymorphism
daixtrose Oct 16, 2024
4d1aeda
enable tests only for top-level build
daixtrose Oct 16, 2024
8b242de
Add build and test instructions
daixtrose Oct 16, 2024
b654f5d
add addition check of side effect
daixtrose Oct 16, 2024
3ceb45e
Add code explanations in README
daixtrose Oct 16, 2024
a661554
Add bdd test example
daixtrose Oct 16, 2024
9e220f1
add CI for polymorphism
daixtrose Oct 16, 2024
8b1b7f6
fix wrong comment on closing namespace
daixtrose Oct 16, 2024
5ecc7d1
restructure CI commands
daixtrose Oct 16, 2024
56728e2
Merge branch 'add-first-concepts-example' of https://github.com/daixt…
daixtrose Oct 16, 2024
67cab71
refine test code build
daixtrose Oct 16, 2024
d84f1f2
add [[nodiscard]] to a member function of an interface declaration
daixtrose Oct 16, 2024
74e90e5
Fix typos
daixtrose Oct 16, 2024
dcc6a09
Remove : in title
daixtrose Oct 16, 2024
2e834ac
add coderabbitai configuration
daixtrose Oct 16, 2024
d909b5c
refine include path definitions
daixtrose Oct 16, 2024
55afa89
fix typo
daixtrose Oct 16, 2024
4c937f9
fix typos
daixtrose Oct 16, 2024
850b2eb
add include guards to ipp file
daixtrose Oct 16, 2024
533f18f
fix typo
daixtrose Oct 16, 2024
644c31f
change include directory config to PRIVATE
daixtrose Oct 16, 2024
ff39cad
add WORKING_DIRECTORY to test definition
daixtrose Oct 16, 2024
11e04b0
fix whitespace
daixtrose Oct 16, 2024
917436a
remove blank line
daixtrose Oct 16, 2024
38aa34e
fix typos and text errors
daixtrose Oct 16, 2024
07e948a
trim trailing spaces
daixtrose Oct 16, 2024
e0d3d98
refinements
daixtrose Oct 16, 2024
8cad8a1
add explanation for using ut test framework
daixtrose Oct 16, 2024
ba67fbf
consistent include guards
daixtrose Oct 16, 2024
17c651f
fix include guard
daixtrose Oct 16, 2024
8434ea3
fix typo in include guard
daixtrose Oct 16, 2024
e26e0bb
add comma
daixtrose Oct 16, 2024
4edbbca
add a comment to a build option
daixtrose Oct 16, 2024
d9d524a
extend doc comment for ISuperCoolFeatures
daixtrose Oct 16, 2024
b21206d
add noexcept to all methods in demo classes
daixtrose Oct 16, 2024
5e22a5f
add message to Impl check
daixtrose Oct 16, 2024
0386212
add missing header file
daixtrose Oct 17, 2024
db2bb31
fix typos and blanks
daixtrose Oct 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
language: "en-US"
early_access: false
reviews:
profile: "assertive"
request_changes_workflow: false
high_level_summary: true
poem: true
review_status: true
collapse_walkthrough: false
auto_review:
enabled: true
drafts: false
chat:
auto_reply: true
13 changes: 13 additions & 0 deletions .github/workflows/build_examples.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ jobs:
- uses: actions/checkout@v4
- name: configure
run: cmake -D CMAKE_BUILD_TYPE=Release -B build_example_3 -S example_3

- name: build example_3
run: cmake --build build_example_3 --config Release --target example_3

- name: create example_3 package
run: cmake --build build_example_3 --config Release --target package

- name: check content of *.deb package
id: check-packages
working-directory: build_example_3
Expand All @@ -31,6 +34,7 @@ jobs:
echo "RPM_PACKAGE_FILENAME=${RPM_PACKAGE_FILENAME}" >> $GITHUB_OUTPUT
echo "Checking content of '$DEBIAN_PACKAGE_FILENAME'"
dpkg -c ${DEBIAN_PACKAGE_FILENAME}

- name: Release
uses: softprops/action-gh-release@v2
## if: startsWith(github.ref, 'refs/tags/')
Expand All @@ -40,6 +44,15 @@ jobs:
build_example_3/${{ steps.check-packages.outputs.DEBIAN_PACKAGE_FILENAME }}
token: ${{ secrets.GITHUB_TOKEN }}
tag_name: latest

- name: Configure polymorphism
run: cmake -D CMAKE_BUILD_TYPE=Debug -B build_polymorphism -S polymorphism

- name: Build polymorphism
run: cmake --build build_polymorphism --config Debug --target all --parallel

- name: Test polymorphism
run: ctest --test-dir build_polymorphism --output-on-failure --build-config Debug
# TODO: check https://github.com/marketplace/actions/cmake-swiss-army-knife
# TODO: check https://github.com/actions/starter-workflows/blob/9f1db534549e072c20d5d1a79e0a4ff45a674caf/ci/cmake-multi-platform.yml#L20

20 changes: 20 additions & 0 deletions polymorphism/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
cmake_minimum_required(VERSION 3.29)

project(polymorphism VERSION 1.0.0 LANGUAGES CXX)

option(POLYMORPHISM_ENABLE_TESTING "enable testing for polymorphism" ${PROJECT_IS_TOP_LEVEL})

add_executable(${PROJECT_NAME}
src/consume_class_with_interface.cpp
src/consume_class_that_adheres_to_concept.cpp
src/main.cpp
)

target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_23)
target_include_directories(${PROJECT_NAME} PUBLIC include)
target_include_directories(${PROJECT_NAME} PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/src)

if(POLYMORPHISM_ENABLE_TESTING)
enable_testing()
add_subdirectory(test)
endif()
192 changes: 192 additions & 0 deletions polymorphism/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
## Summary

This example shows two different ways to implement [polymorphism](https://en.wikipedia.org/wiki/Polymorphism_(computer_science)) in C++:

1. The classic approach using an [abstract class](https://en.cppreference.com/w/cpp/language/abstract_class) which is used to define an interface and from which the implementation inherits.
2. The modern approach using [concepts](https://en.cppreference.com/w/cpp/language/constraints) to declare features a type must have.

In addition, this directory contains an example about how to perform unit tests using the [ut testing framework](https://github.com/boost-ext/ut).

## Build Instructions

### Preparation

Create a subdirectory `build_polymorphism` and prepare the build system for the source located in `polymorphism` in this subdirectory: For release builds use

```bash
cmake -D CMAKE_BUILD_TYPE=Release -B build_polymorphism -S polymorphism
```
For debug builds use

```bash
cmake -D CMAKE_BUILD_TYPE=Debug -B build_polymorphism -S polymorphism
```


### Building the `polymorphism` project

Call `cmake` from the top level like this:

```bash
cmake --build build_polymorphism --config Release --target polymorphism --parallel
```

## Test Instructions

### Build the tests

```bash
VERBOSE=1 cmake --build build_polymorphism --clean-first --config Debug --target all --parallel
```

### Run the tests

```bash
ctest --test-dir build_polymorphism --output-on-failure --build-config Debug
```
Comment on lines +35 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Concise test instructions.

The test instructions are clear and provide the necessary commands to build and run the tests.

Consider adding the following to enhance this section:

  1. A brief explanation of what the tests cover and why they are important.
  2. Information on how to interpret the test results.
  3. Guidance on what to do if tests fail.


## Code Explanations

### Classic Approach

In classic C++, interfaces are defined via [abstract base classes](https://en.cppreference.com/w/cpp/language/abstract_class) from which the implementation (publicly) inherits. In this example, we have defined such an abstract class:

```c++
class ISuperCoolFeatures {
public:
virtual std::string coolFeature() const = 0; // Pure virtual function
virtual void set(std::string s) = 0; // Pure virtual function
virtual ~ISuperCoolFeatures() = default; // Virtual destructor
};
```

Instead of defining the member function body (which is possible in a regular base class), the member functions of an abstract interface class are declared "pure virtual" by adding `= 0` to their declaration.

Any so-called **implementation** of the interface publicly inherits from this abstract base class and then declares and defines member functions that match the declarations of the pure virtual functions in the abstract interface class:

```c++
class Impl
: public ISuperCoolFeatures {

// ... private members and member functions omitted ...

public:
std::string coolFeature() const override { return s_; }
void set(std::string s) override
{
// ... implementation omitted ...
}
};
```

A variable (aka instantiation) of type `Impl` can be passed as argument to any function that expects an argument of its interface type `ISuperCoolFeatures`. A variable (aka instatiation) of type `Impl` can be passed as argument to any function that expects an argument of its interface type `ISuperCoolFeatures`.

In our example we define a function which has one argument of type `ISuperCoolFeatures`, and returns a `std::string`:

```c++
std::string consume(ISuperCoolFeatures& f);
```

We then pass an argument of type `Impl` to it, e.g. like this:

```c++
Impl i;
consume(i);
```

Since it is best practice, the example code puts the type and function definitions into namespaces.

### Modern Approach

In modern C++, interfaces can also be defined via [concepts](https://en.cppreference.com/w/cpp/language/constraints), which can be used to declare what features a concrete class must have. It is slightly more powerful in what can be described.

It also has a great advantage that it is non-intrusive, i.e. it does not require a concrete class to inherit from a certain interface class. This is of advantage when you are not owner of the code of a class that you want to use and cannot change its declaration and definition.

This means that objects of completely unrelated classes can be passed as arguments to a member function (aka method) or a free function, if they all have certain memeber functions defined.

From a code structuring point of view, this means that the cohesion between different parts of the code is reduced.

In this example, we have defined such an interface specification as a concept:

```c++
template <typename T>
concept has_super_cool_features = requires(T t, std::string s) {
{ t.coolFeature() } -> std::convertible_to<std::string>;
{ t.set(s) } -> std::same_as<void>;
};
```

We then declare a function that takes arguments whose type adheres to the constraints defined by the concept:

```c++
std::string consume(has_super_cool_features auto& s);
```

and can use it in the very same way like in the classic case:

```c++
Impl i;
consume(i);
```

In this approach, our implementation is defined **without** inheritance:

```c++
class Impl {

// ...

public:
std::string coolFeature() const { /* ... some code ... */ }
void set(std::string s)
{
// code omitted
}
};
```

To catch errors in the implementation before the class gets uses somewhere else in the code, it suffices to check against the concept like this:

```c++
static_assert(has_super_cool_features<Impl>);
```

Please note, that this modern approach with its advantages in code decoupling, flexibility and a more powerful description of its features comes at a price:

Functions that use `<some concept> auto` as parameter type are *generic* and this requires either their implicit or their explicit instantiation.

Implicit instantiation of the function can take place if its **definition** is visible at the point of usage.

In this example, we use explicit instantiation to show the possibility to clearly separate the definition (implementation) of the generic function from its declaration.

You can find the definition of `consume` in `src/consume_class_that_adheres_to_concept.ipp` which is then included into `src/consume_class_that_adheres_to_concept.cpp`. In the latter file we also place all explicit instantiations of `consume`:

```c++
// explicit template instantiation
template std::string consume<Impl>(Impl&);
```

Als an alternative, the *definition* of `consume` could be in the header `include/polymorphism/consume_class_that_adheres_to_concept.hpp`. Then the explicit template instantiation is no longer required.

## Testing

In this example, we use [µt](https://github.com/boost-ext/ut) as testing framework.

The one-header version of it is pulled into the project via the CMake call

```cmake
FetchContent_Declare(ut
URL https://raw.githubusercontent.com/boost-ext/ut/refs/heads/master/include/boost/ut.hpp
DOWNLOAD_NO_EXTRACT TRUE
SOURCE_DIR ut
)
FetchContent_MakeAvailable(ut)
```

and the directory to which the header is downloaded is added to the search path set for the build of the test executables:

```cmake
target_include_directories(test_consume PUBLIC "${ut_SOURCE_DIR}")
```

Please refer to [this slidedeck](https://boost-ext.github.io/ut/denver-cpp-2019/#/) for further documentation of this test library.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#ifndef CONSUME_CLASS_THAT_ADHERES_TO_CONCEPT_HPP
#define CONSUME_CLASS_THAT_ADHERES_TO_CONCEPT_HPP

#include <polymorphism/has_super_cool_features.hpp>

namespace modern {

// declaration with concept constraint
std::string consume(has_super_cool_features auto& s);
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Enhance function documentation

While the function declaration looks good and uses modern C++ features as intended, the documentation could be improved for better clarity.

Consider using a Doxygen-style comment for better documentation. Here's a suggested improvement:

/**
 * @brief Consumes an object that adheres to the has_super_cool_features concept.
 * @param s Reference to an object satisfying the has_super_cool_features concept.
 * @return A string result after consuming the object.
 */
std::string consume(has_super_cool_features auto& s);


} // namespace modern

#endif // CONSUME_CLASS_THAT_ADHERES_TO_CONCEPT_HPP
12 changes: 12 additions & 0 deletions polymorphism/include/polymorphism/consume_class_with_interface.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#ifndef CONSUME_CLASS_WITH_INTERFACE_HPP
#define CONSUME_CLASS_WITH_INTERFACE_HPP

#include <polymorphism/i_super_cool_features.hpp>

namespace classic {

std::string consume(ISuperCoolFeatures& f);

} // namespace classic
Comment on lines +6 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Namespace and function declaration look good. Consider adding a brief comment.

The use of the "classic" namespace and the function declaration for consume are well-structured. To improve clarity, consider adding a brief comment explaining the purpose of the consume function. For example:

namespace classic {

// Consumes an object implementing the ISuperCoolFeatures interface
// and returns a string representation of the result.
std::string consume(ISuperCoolFeatures& f);

} // namespace classic

This addition would provide context for developers who might use this function in the future.


#endif // CONSUME_CLASS_WITH_INTERFACE_HPP
17 changes: 17 additions & 0 deletions polymorphism/include/polymorphism/has_super_cool_features.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#ifndef EXAMPLE_5_FEATURES_HPP
#define EXAMPLE_5_FEATURES_HPP

#include <string>

namespace modern {

/// @brief A concept defintion describing a class or struct that has two member functions.
template <typename T>
concept has_super_cool_features = requires(T t, std::string s) {
{ t.coolFeature() } -> std::convertible_to<std::string>;
{ t.set(s) } -> std::same_as<void>;
};
Comment on lines +8 to +13
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Concept definition is well-structured

The has_super_cool_features concept is well-defined and correctly implements the requirements for the two member functions. The use of C++20 features like the requires clause and type constraints is appropriate.

Some suggestions for improvement:

  1. Consider using a more formal name for the concept in a production environment.
  2. Expand the comment to provide more details about the expected behavior of coolFeature() and set().
  3. If this is part of a larger API, consider adding static_assert statements in the implementation to provide more user-friendly error messages when the concept is not satisfied.


} // namespace modern

#endif // EXAMPLE_5_FEATURES_HPP
18 changes: 18 additions & 0 deletions polymorphism/include/polymorphism/i_super_cool_features.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#ifndef ISUPERCOOLFEATURES_HPP
#define ISUPERCOOLFEATURES_HPP

#include <string>

namespace classic {

/// @brief A typical interface definition via a class with pure virtual functions/methods.
class ISuperCoolFeatures {
public:
[[nodiscard]] virtual std::string coolFeature() const = 0; // Pure virtual function
virtual void set(std::string s) = 0; // Pure virtual function
virtual ~ISuperCoolFeatures() = default; // Virtual destructor
};
Comment on lines +15 to +20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

LGTM: Well-designed interface with proper use of C++ features.

The ISuperCoolFeatures interface is well-structured, following C++ best practices for abstract classes. The use of pure virtual functions and a virtual destructor is correct. The [[nodiscard]] attribute on coolFeature() is a good practice to prevent accidental ignoring of the return value.

Consider adding the override specifier to the destructor for consistency and to make the interface more robust:

virtual ~ISuperCoolFeatures() override = default;

This change would ensure that if the interface is inherited from another base class in the future, the destructor will be correctly overridden.


} // namespace classic

#endif // ISUPERCOOLFEATURES_HPP
26 changes: 26 additions & 0 deletions polymorphism/include/polymorphism/impl_with_interface.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#ifndef IMPL_WITH_INTERFACE_HPP
#define IMPL_WITH_INTERFACE_HPP

// make the inteface defintion visible
#include <polymorphism/i_super_cool_features.hpp>

#include <string>

namespace classic {

class Impl
: public ISuperCoolFeatures {
private:
std::string s_ { "<default value>" }; // with default member initializer (C++11)
Comment on lines +11 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

LGTM: Class declaration and member initialization are well-implemented.

The Impl class correctly inherits from ISuperCoolFeatures, and the private member s_ uses a C++11 default member initializer.

Consider adding a brief comment explaining the purpose of the s_ member variable for improved code documentation. For example:

private:
    std::string s_ { "<default value>" }; // Stores the current feature value


public:
std::string coolFeature() const override { return s_; }
void set(std::string s) override
{
s_ = std::move(s);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

LGTM: Public methods are correctly implemented.

The coolFeature() and set(std::string s) methods properly override the interface methods. The use of std::move in the set method is efficient for string assignment.

Consider marking the set method's parameter as const std::string& if you don't intend to modify it before moving:

void set(const std::string& s) override
{
    s_ = s;  // The move will be implicitly applied here
}

This allows for more flexible use of the method with both lvalues and rvalues.


} // namespace classic

#endif // IMPL_WITH_INTERFACE_HPP
27 changes: 27 additions & 0 deletions polymorphism/include/polymorphism/impl_without_interface.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#ifndef IMPL_WITHOUT_INTERFACE_HPP
#define IMPL_WITHOUT_INTERFACE_HPP

#include <polymorphism/has_super_cool_features.hpp>

#include <string>

namespace modern {

class Impl {
Comment on lines +8 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider using a more descriptive class name.

While Impl is concise, it might be too generic in a larger codebase. Consider a more descriptive name that reflects the class's purpose or functionality, such as ModernFeatureImpl or ConceptBasedImpl.

private:
std::string s_ { "<default value>" }; // with default member initializer (C++11)

public:
std::string coolFeature() const { return s_; }
void set(std::string s)
{
s_ = std::move(s);
}
};

// Check if the class adheres to the concept (i.e. has the interface we want it to have).
static_assert(has_super_cool_features<Impl>);

} // namespace modern

#endif // IMPL_WITHOUT_INTERFACE_HPP
Loading