szaszm commented on a change in pull request #1225:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1225#discussion_r769622892



##########
File path: libminifi/include/utils/TimeUtil.h
##########
@@ -197,6 +193,100 @@ inline bool getDateTimeStr(int64_t unix_timestamp, 
std::string& date_time_str) {
   return true;
 }
 
+namespace details {
+
+template<class Duration>
+bool unit_matches(const std::string&) {
+  return false;
+}
+
+template<>
+inline bool unit_matches<std::chrono::nanoseconds>(const std::string& unit) {
+  return unit == "ns" || unit == "nano" || unit == "nanos" || unit == 
"nanoseconds";
+}
+
+template<>
+inline bool unit_matches<std::chrono::microseconds>(const std::string& unit) {
+  return unit == "us" || unit == "micro" || unit == "micros" || unit == 
"microseconds" || unit == "microsecond";
+}
+
+template<>
+inline bool unit_matches<std::chrono::milliseconds>(const std::string& unit) {
+  return unit == "msec" || unit == "ms" || unit == "millisecond" || unit == 
"milliseconds" || unit == "msecs" || unit == "millis" || unit == "milli";
+}
+
+template<>
+inline bool unit_matches<std::chrono::seconds>(const std::string& unit) {
+  return unit == "sec" || unit == "s" || unit == "second" || unit == "seconds" 
|| unit == "secs";
+}
+
+template<>
+inline bool unit_matches<std::chrono::minutes>(const std::string& unit) {
+  return unit == "min" || unit == "m" || unit == "mins" || unit == "minute" || 
unit == "minutes";
+}
+
+template<>
+inline bool unit_matches<std::chrono::hours>(const std::string& unit) {
+  return unit == "h" || unit == "hr" || unit == "hour" || unit == "hrs" || 
unit == "hours";
+}
+
+template<>
+inline bool unit_matches<std::chrono::days>(const std::string& unit) {
+  return unit == "d" || unit == "day" || unit == "days";
+}
+
+
+template<class TargetDuration, class SourceDuration>
+std::optional<TargetDuration> cast_if_unit_matches(const std::string& unit, 
const int64_t value) {
+  if (unit_matches<SourceDuration>(unit)) {
+    return std::chrono::duration_cast<TargetDuration>(SourceDuration(value));
+  } else {
+    return std::nullopt;
+  }
+}
+
+template<class TargetDuration, typename... T>
+std::optional<TargetDuration> cast_to_matching_unit(std::string& unit, const 
int64_t value) {
+  std::optional<TargetDuration> result;
+  ((result = cast_if_unit_matches<TargetDuration, T>(unit, value)) || ...);
+  return result;
+}
+
+inline bool get_unit_and_value(const std::string& input, std::string& unit, 
int64_t& value) {
+  std::stringstream input_stream(input);
+  input_stream >> value >> unit;
+  std::transform(unit.begin(), unit.end(), unit.begin(), ::tolower);
+  return !input_stream.fail();
+}
+
+}  // namespace details
+
+template<class TargetDuration>
+std::optional<TargetDuration> StringToDuration(const std::string& input) {
+  std::string unit;
+  int64_t value;
+  if (!details::get_unit_and_value(input, unit, value))
+    return std::nullopt;
+
+  return details::cast_to_matching_unit<TargetDuration,
+    std::chrono::nanoseconds,
+    std::chrono::microseconds,
+    std::chrono::milliseconds,
+    std::chrono::seconds,
+    std::chrono::minutes,
+    std::chrono::hours,
+    std::chrono::days>(unit, value);
+}
+
+
+inline std::chrono::milliseconds getMillisecondsSinceUnixEpoch(const 
std::chrono::time_point<std::chrono::system_clock> time_point) {
+  return 
std::chrono::duration_cast<std::chrono::milliseconds>(time_point.time_since_epoch());
+}
+
+inline std::chrono::time_point<std::chrono::system_clock> 
fromMillisecondsSinceUnixEpoch(const std::chrono::milliseconds 
milliseconds_since_unix_epoch) {
+  return std::chrono::time_point<std::chrono::system_clock>() + 
milliseconds_since_unix_epoch;
+}

Review comment:
       I think we can assume that the unix epoch is the system_clock epoch, but 
I didn't know that it wasn't universal before C++20.
   For me, `duration_cast<milliseconds>(time_point.time_since_epoch())` reads 
as "the time since epoch, in milliseconds", which is about the same as the 
function name, that's why I didn't see the added value.
   Similarly `time_point{} + milliseconds_since_epoch` reads as "epoch + 
milliseconds since epoch", so to me it's obvious what's happening, and it's 
better described by the expression than the function name IMO.
   
   If you disagree and see additional value here, then I won't stand in the 
way. I leave my arguments here, but you should make the call.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to