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 all 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

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

project(polymorphism VERSION 1.0.0 LANGUAGES CXX)

# Enable testing by default when this is the top-level project
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} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src)

if(POLYMORPHISM_ENABLE_TESTING)
message(STATUS "Polymorphism testing enabled")
enable_testing()
add_subdirectory(test)
else()
message(STATUS "Polymorphism testing disabled")
endif()
193 changes: 193 additions & 0 deletions polymorphism/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
# C++ Polymorphism Example

## 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).

Comment on lines +1 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)

Great introduction and summary!

The introduction provides a clear and concise overview of the two approaches to polymorphism in C++ and mentions the use of the ut testing framework. This sets the stage well for the rest of the document.

Consider adding a brief explanation of why understanding these two approaches is important for C++ developers. This could help motivate readers to explore the examples further.

## 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
```
Comment on lines +12 to +33
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)

Clear and comprehensive build instructions.

The build instructions are well-structured and provide clear commands for both release and debug builds. This will be very helpful for users trying to build the project.

Consider making the following improvements:

  1. Add a brief explanation of why CMake is being used and its benefits for this project.
  2. Include information about any prerequisites or dependencies needed to build the project.
  3. Add a blank line before and after the code block at line 20 for consistency with Markdown best practices:
 cmake -D CMAKE_BUILD_TYPE=Release -B build_polymorphism -S polymorphism
+

For debug builds use
+

🧰 Tools
🪛 Markdownlint

20-20: null
Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


## 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 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`.

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 member 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&);
```

As 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.
Comment on lines +49 to +170
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)

Comprehensive code explanations for both approaches.

The explanations for both the classic and modern approaches to polymorphism are detailed and well-structured. The code snippets effectively illustrate the concepts.

Consider making the following improvements:

  1. Add comments to the code snippets to highlight key points or explain complex parts.
  2. Provide a brief comparison of the pros and cons of each approach at the end of this section.
  3. Include a small, complete example that demonstrates both approaches side-by-side for easy comparison.


## Testing

In this example, I use [µt](https://github.com/boost-ext/ut) as testing framework for no good reason. It looks promising and lightweight, and it was written by [Krzysztof Jusiak](https://github.com/krzysztof-jusiak) whom I adore for his outstanding C++ programming skills and his [C++ Tip of The Week](https://github.com/tip-of-the-week/cpp) collection of crazy C++ riddles.

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.
Comment on lines +172 to +193
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)

Informative testing section with helpful CMake snippets.

The testing section provides clear information about using the µt testing framework, including CMake code snippets for including the framework. The link to further documentation is a valuable addition.

Consider making the following improvements:

  1. Explain why µt was chosen over other testing frameworks. This context could be helpful for users who are new to C++ testing or are considering different options.
  2. Provide a brief example of how to write a simple test using µt, to give readers a quick start.
  3. Add information on how to add new tests to the project.

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef POYMORPHISM_CONSUME_CLASS_THAT_ADHERES_TO_CONCEPT_HPP
#define POYMORPHISM_CONSUME_CLASS_THAT_ADHERES_TO_CONCEPT_HPP

#include <polymorphism/has_super_cool_features.hpp>

#include <string>

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 // POYMORPHISM_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 POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP
#define POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP
Comment on lines +1 to +2
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 unique prefix for the include guard macro.

While the typo has been fixed, consider using a more specific prefix to ensure uniqueness and prevent potential conflicts with other libraries. For example:

-#ifndef POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP
-#define POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP
+#ifndef CPLUSPLUS_PRIMER_POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP
+#define CPLUSPLUS_PRIMER_POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP

 // ... (rest of the file)

-#endif // POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP
+#endif // CPLUSPLUS_PRIMER_POLYMORPHISM_CONSUME_CLASS_WITH_INTERFACE_HPP

This change would make the include guard more specific to your project.

Also applies to: 12-12


#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 // POLYMORPHISM_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 POYMORPHISM_HAS_SUPER_COOL_FEATURES_HPP
#define POYMORPHISM_HAS_SUPER_COOL_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 // POYMORPHISM_HAS_SUPER_COOL_FEATURES_HPP
24 changes: 24 additions & 0 deletions polymorphism/include/polymorphism/i_super_cool_features.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#ifndef POLYMORPHISM_I_SUPER_COOL_FEATURES_HPP
#define POLYMORPHISM_I_SUPER_COOL_FEATURES_HPP

#include <string>

namespace classic {

/// @brief A typical interface definition via a class with pure virtual functions/methods.
///
/// This interface demonstrates classic polymorphism in C++.
///
/// @details
/// - coolFeature(): Returns a string representing some cool feature.
/// - set(std::string s): Sets some internal state of the implementing class.
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 // POLYMORPHISM_I_SUPER_COOL_FEATURES_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 POLYMORPHISM_IMPL_WITH_INTERFACE_HPP
#define POLYMORPHISM_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 noexcept override { return s_; }
void set(std::string s) noexcept override
{
s_ = std::move(s);
}
Comment on lines +17 to +21
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 implementation.

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

Consider changing the set method's parameter to const std::string& for more flexibility:

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

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

};
Comment on lines +16 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

LGTM: Public methods are correctly implemented.

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

Consider changing the set method's parameter to const std::string& for more flexibility:

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

This allows for more flexible use of the method with both lvalues and rvalues, potentially improving performance in some cases.


} // namespace classic

#endif // POLYMORPHISM_IMPL_WITH_INTERFACE_HPP
Loading