Refactor buildDate() to use string_views #51
Conversation
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
This function was previously untested.
std::string_view is the modern C++ replacement for null-terminated char pointers. In this specific function, it has two advantages: First, instead of constructing four new strings (dateStr, yearStr, monthStr, and dayStr), it is possible to use slices that reference parts of the original string_view instead. This saves four copies. Second, the initialization of the "months" array can be moved to compile-time, since std::string_view is constexpr (unlike std::string). This commit also replaces atoi with from_chars, as atoi requires a null-terminated string. This is beneficial, because from_chars provides an error code indicating whether or not the parsing was successful.
This is recommended by ES.28 of the C++ Core Guidelines, because month is not intended to be modified after initialization.
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.
Yo
2E29
u 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.
Pull Request Template
Summary
Build::buildDate()utility function to usestd::string_views for performance and safety, and adds test cases to verify the functionality.Changes
Build::buildDate()Build::buildDate()to usestd::string_viewas a parameter and for intermediate variablesstatic constarray ofstd::stringto be astatic constexprarray ofstd::string_view.Motivation
Build::buildDate()function is called.static constexpr std::string_views are initialized at compile-time instead of run-time.Testing
Build::buildDate(), so I added one that tests the date "Jan 2, 1970".Checklist
Additional Comments
std::strings intostd::string_views but I'm unsure of whether this'd be welcome, so I haven't included commits updating the style guide.