Add C++ refactoring examples (C++23/26)#118
Open
chibi-dogs wants to merge 9 commits into
Open
Conversation
…se idiomatic language features
…t instead to apply discount
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR ports some of the existing examples in Java to C++. I have added before and after examples for the following 3 techniques:
All examples compile when tested with clang trunk and using C++26. The
beforefiles are a direct, idiomatic C++ translation of the Java originals. Theafterfiles apply the refactoring and additionally adopt several modern C++ idioms where I felt appropriate.Design decisions & deviations from Java originals
I was having trouble porting some of the solutions to C++ since many things that may be considered idiomatic in Java don't necessarily have 1 to 1 mappings in C++. I made the following decisions:
Substitute Algorithm
before: rawif-chain returning""on failure — true to the Java originalafter: replaces the chain withstd::ranges::find_first_ofover astatic constexpr std::array<std::string_view>, and returnsstd::optional<std::string>instead of an empty string sentinel.std::optional. I think that using rangesfind_first_ofis really nice since it expresses the algorithm's intent quite clearly.Extract Method – Isolate Switch
before: raw loop +switchon an enum, applying a discount factorafter: replaces theswitchwithstd::variant+std::visitover typed discount structs, each holding astatic constexpr double factor; total is computed viastd::ranges::fold_leftstd::variant+std::visitwhich also would fit Replace Conditional with Polymorphism quite nicely.Extract Method (basic)
std::cout << std::format() << std::endlwould work even for older versions of the language and compilers so it might be a better choice. But I decided to go ahead with std::print since the name carries the intent a lot better in my opinion.Notes
std::print,std::ranges::fold_left,std::ranges::find_first_of,std::span)[[nodiscard]]when appropriate even though it adds some "noise". I added it to the signature of all functions that return values since looking at clang tidy complaining about it was stressing me out 😅Happy to discuss these design decisions! 😄