Skip to content
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

refactor: change DEF_EX macro's underlying implementation to template #1939

Merged

Conversation

shenlebantongying
Copy link
Collaborator

@shenlebantongying shenlebantongying commented Nov 12, 2024

Fun stuff :)

@shenlebantongying shenlebantongying changed the title refactor: change DEF_EX macro to template refactor: change DEF_EX macro's underlying implementation to template Nov 12, 2024
@shenlebantongying shenlebantongying force-pushed the refactor/super-ex-temp branch 4 times, most recently from 23cffd4 to 8c93b17 Compare November 12, 2024 05:20
@shenlebantongying shenlebantongying enabled auto-merge (rebase) November 12, 2024 05:21
@shenlebantongying shenlebantongying merged commit 4758f9e into xiaoyifang:staged Nov 12, 2024
7 checks passed
Copy link

@shenlebantongying shenlebantongying deleted the refactor/super-ex-temp branch November 12, 2024 05:31
@xiaoyifang
Copy link
Owner

xiaoyifang commented Nov 12, 2024

template and extra fmt::format? not easy to understand compared to #define.
the previous version is ok.

@shenlebantongying
Copy link
Collaborator Author

shenlebantongying commented Nov 12, 2024

Now it is easier to modify them. Macro don't have types, but template code have some types.

In IDEs, the code can be edited like normal code with inline hints, type warnings...

And there are no warnings anymore.


The single line fmt::format is technically better

From

std::string( exDescription ) + " " + value_

to

fmt::format( "{} {}", description, message_ )

One std::string construction and " " concatenation are removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants