perf fix Time #142
perf fix Time #142
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #142 +/- ##
===========================================
+ Coverage 64.06% 64.07% +0.01%
===========================================
Files 1079 1079
Lines 55679 55688 +9
Branches 4118 4124 +6
===========================================
+ Hits 35668 35680 +12
+ Misses 20011 20008 -3 ☔ View full report in Codecov by Sentry. |
| throw BadTime("Unkown format for time: " + s); | ||
| } else { | ||
| throw SeriousBug("Unhandled time format!"); |
There was a problem hiding this comment.
Why is there a need for two different exceptions? I understand the intent of those exception types as follows:
BadTimeis best described as a usage error, as in it was called asTime("ABCD")SeriousBugrepresents an implementation error such asTime("20:11:24")not getting parsed properly.
Is that second throw giving any additional benefits?
|
|
||
| // DIGITS: "^-?[0-9]+$" | ||
| // FLOAT: "^-?[0-9]*\\.[0-9]+$" | ||
| enum class TimeFormat { UNKOWN, OTHER, DIGITS, DECIMAL }; |
There was a problem hiding this comment.
UNKNOWN is treated as erroneous input, it go stronger in the name and call it INVALID
| ss = sec; | ||
| } else if (format == TimeFormat::OTHER) { | ||
| std::smatch m; | ||
| if (std::regex_match(s, m, hhmmss_)) { |
There was a problem hiding this comment.
This is still using a regex.
| ss += 60 * (mm + 60 * (hh + 24 * dd)); | ||
| if (s[0] == '-') { | ||
| ss = -ss; | ||
| } else if (std::regex_match(s, m, ddhhmmss_)) { |
There was a problem hiding this comment.
This is still using a regex.
| for (auto i = start; i < time.length(); i++) { | ||
| if (time[i] == '.') { | ||
| if (hasDecimal || i == time.length() - 1) { return TimeFormat::UNKOWN; } | ||
| hasDecimal = true; | ||
| } else if (isdigit(time[i]) == 0) { | ||
| return TimeFormat::OTHER; | ||
| } else { | ||
| hasDigit = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
I think this can be done with a single pass over the input string and then return the typeid of the found variant (pretty similar to what you are doing already) plus up to 5 tokens, 1 token for the sign and 4 for positive integers (day, hour, minute, second).
Subsequent code can then validate on this result, e.g. was the extended flag passed.
I am thinking along the lines of:
struct tokenized_time {
FormatType type;
std::string_view sign;
std::string_view integers[4];
};
try to avoid
std::regex