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

Let std::format do the work #721

Closed
wants to merge 4 commits into from

Conversation

tobiolo
Copy link
Collaborator

@tobiolo tobiolo commented Sep 22, 2024

No description provided.

Ubuntu 22.04 LTS does not support the C++20 features, but
Ubunntu 24.04 LTS does.
C++20 features are supported starting from MacOS 13.3.

t = s;
}
void SetNum(double d) { t = std::format("{:g}", d); }
Copy link
Owner

Choose a reason for hiding this comment

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

This is unlikely to produce the same formatting as the code below. The existing code does a lot of specific things to achieve a precise formatting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But is this stuff really necessary?

Copy link
Owner

Choose a reason for hiding this comment

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

Most code that is there, especially if it does something which appears over complicated or does extra things, does so for a reason, yes.

Simplifying it is good, if you can indeed verify that the simplified version does the same thing, and if it doesn't, that should only be done if its an improvement.

For example, from the comments you can see that this implements a variable length float format, that avoids low precisions last digits and/or scientific notation.. basically a friendly human-like representation. There's no way to get that formatting with the standard functions that I know of.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your explanation! Thus I close this PR.

@tobiolo tobiolo closed this Sep 28, 2024
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