From 9a34c67e8a1029f55a8a9730fb418921fa8efbcf Mon Sep 17 00:00:00 2001
From: Emanuele Danovaro <emanuele.danovaro@ecmwf.int>
Date: Wed, 27 Sep 2023 13:05:28 +0100
Subject: [PATCH] Time parsing - added support for fraction of an hour

---
 src/eckit/types/Time.cc  | 72 ++++++++++++++++++++++------------------
 src/eckit/types/Time.h   |  4 +--
 tests/types/test_time.cc | 39 +++++++++++++++++-----
 3 files changed, 73 insertions(+), 42 deletions(-)

diff --git a/src/eckit/types/Time.cc b/src/eckit/types/Time.cc
index 68eb61f94..0c0a7464b 100644
--- a/src/eckit/types/Time.cc
+++ b/src/eckit/types/Time.cc
@@ -29,10 +29,10 @@ inline void printTime(std::ostream& s, long n) {
     s << n;
 }
 
-Time::Time(long seconds) :
+Time::Time(long seconds, bool extended) :
     seconds_(seconds) {
-    if (seconds >= 86400 || seconds < 0) {
-        std::string msg = "Time in seconds must be positive and cannot exceed 86400, seconds: ";
+    if ((seconds >= 86400 && !extended) || seconds < 0) {
+        std::string msg = "Time in seconds must be positive and less than 86400 seconds (24h): ";
         Translator<long, std::string> t;
         msg += t(seconds);
         throw BadTime(msg);
@@ -45,10 +45,10 @@ Time::Time(const std::string& s, bool extended) {
     
     if (std::regex_match (s, m, std::regex("^[0-9]+$"))) { // only digits
         long t = std::stol(s);
-        if (s.length() < 3) {     // cases: h, hh
+        if (extended || s.length() <= 2) {     // cases: h, hh, (or hhh..h for step parsing)
             hh = t;
         } else {
-            if (s.length() < 5) { // cases: hmm, hhmm
+            if (s.length() <= 4) { // cases: hmm, hhmm
                 hh = t / 100;
                 mm = t % 100;
             } else {              // cases: hmmss, hhmmss
@@ -59,45 +59,53 @@ Time::Time(const std::string& s, bool extended) {
         }
     }
     else {
-        if (std::regex_match (s, m, std::regex("^([0-9]+):([0-5]?[0-9])(:[0-5]?[0-9])?$"))) {
-            for (int i=1; i<m.size(); i++) {
-                if (m[i].matched) {
-                    switch (i) {
-                        case 1: hh = std::stol(m[i].str()); break;
-                        case 2: mm = std::stol(m[i].str()); break;
-                        case 3: std::string aux = m[i].str();
-                                aux.erase(0,1);
-                                ss = std::stol(aux); break;
-                    }
-                }
-            }
+        if (std::regex_match (s, m, std::regex("^[0-9]*\\.[0-9]+$"))) { // floating point (hours)
+            long sec = std::round(std::stod(s)*3600);
+            hh = sec/3600;
+            sec -= hh*3600;
+            mm = sec/60;
+            sec -= mm*60;
+            ss = sec;
         }
         else {
-            if (std::regex_match (s, m, std::regex("^([0-9]+[dD])?([0-9]+[hH])?([0-9]+[mM])?([0-9]+[sS])?$"))) {
+            if (std::regex_match (s, m, std::regex("^([0-9]+):([0-5]?[0-9])(:[0-5]?[0-9])?$"))) {
                 for (int i=1; i<m.size(); i++) {
                     if (m[i].matched) {
-                        std::string aux = m[i].str();
-                        aux.pop_back();
-                        long t = std::stol(aux);
                         switch (i) {
-                            case 1: dd = t; break;
-                            case 2: hh = t; break;
-                            case 3: mm = t; break;
-                            case 4: ss = t;
+                            case 1: hh = std::stol(m[i].str()); break;
+                            case 2: mm = std::stol(m[i].str()); break;
+                            case 3: std::string aux = m[i].str();
+                                    aux.erase(0,1);
+                                    ss = std::stol(aux); break;
                         }
                     }
                 }
-                if (extended) {
+            }
+            else {
+                if (std::regex_match (s, m, std::regex("^([0-9]+[dD])?([0-9]+[hH])?([0-9]+[mM])?([0-9]+[sS])?$"))) {
+                    for (int i=1; i<m.size(); i++) {
+                        if (m[i].matched) {
+                            std::string aux = m[i].str();
+                            aux.pop_back();
+                            long t = std::stol(aux);
+                            switch (i) {
+                                case 1: dd = t; break;
+                                case 2: hh = t; break;
+                                case 3: mm = t; break;
+                                case 4: ss = t;
+                            }
+                        }
+                    }
                     ss += 60 * (mm + 60 * (hh + 24 * dd));
                     dd =  ss / 86400;
                     hh = (ss /  3600) % 24;
                     mm = (ss /    60) % 60;
                     ss =  ss          % 60;
+                } else {
+                    std::string msg = "Wrong input for time: ";
+                    msg += s;
+                    throw BadTime(msg);
                 }
-            } else {
-                std::string msg = "Wrong input for time: ";
-                msg += s;
-                throw BadTime(msg);
             }
         }
     }
@@ -134,9 +142,9 @@ Time& Time::operator=(const Time& other) {
     return *this;
 }
 
-Time::Time(long hh, long mm, long ss) :
+Time::Time(long hh, long mm, long ss, bool extended) :
     seconds_(hh * 3600 + mm * 60 + ss) {
-    if (hh >= 24 || mm >= 60 || ss >= 60 || hh < 0 || mm < 0 || ss < 0) {
+    if ((hh >= 24 && !extended) || mm >= 60 || ss >= 60 || hh < 0 || mm < 0 || ss < 0) {
         std::string msg = "Wrong input for time: ";
         Translator<long, std::string> t;
         msg += t(hh);
diff --git a/src/eckit/types/Time.h b/src/eckit/types/Time.h
index 3c8b27488..026f40e1d 100644
--- a/src/eckit/types/Time.h
+++ b/src/eckit/types/Time.h
@@ -32,8 +32,8 @@ typedef double Second;
 class Time {
 
 public:  // methods
-    Time(long, long, long);
-    Time(long seconds = 0);
+    Time(long, long, long, bool extended = false);
+    Time(long seconds = 0, bool extended = false);
     Time(const std::string&, bool extended = false);
 
 #include "eckit/types/Time.b"
diff --git a/tests/types/test_time.cc b/tests/types/test_time.cc
index 878e65d53..8ec5ebfb1 100644
--- a/tests/types/test_time.cc
+++ b/tests/types/test_time.cc
@@ -30,6 +30,20 @@ CASE("Time only digits (hhmmss)") {
     EXPECT(Time(0,0,0) == Time("0000"));
     EXPECT(Time(0,0,0) == Time("00000"));
     EXPECT(Time(0,0,0) == Time("000000"));
+    EXPECT(Time(0,0,0) == Time(0));
+    EXPECT(Time(0,0,1) == Time(1));
+    EXPECT(Time(0,1,0) == Time(60));
+    EXPECT(Time(0,1,1) == Time(61));
+    EXPECT(Time(1,0,0) == Time(3600));
+    EXPECT(Time(1,0,1) == Time(3601));
+    EXPECT(Time(1,1,0) == Time(3660));
+    EXPECT(Time(1,1,1) == Time(3661));
+    EXPECT(Time(2,3,4) == Time(3600*2 + 60*3 + 4));
+    EXPECT_THROWS(Time(24*3600));
+    EXPECT_THROWS(Time(24,0,0));
+    EXPECT_THROWS(Time("24"));
+    EXPECT(Time(24,0,0, true) == Time(24*3600, true));
+    EXPECT(Time(24,0,0, true) == Time("24", true));
 
     EXPECT(Time(2,0,0) == Time("2"));
     EXPECT(Time(2,0,0) == Time("02"));
@@ -38,6 +52,17 @@ CASE("Time only digits (hhmmss)") {
     EXPECT(Time(2,0,0) == Time("20000"));
     EXPECT(Time(2,0,0) == Time("020000"));
 
+    EXPECT(Time(20,0,0) == Time("20"));
+    EXPECT(Time(20,0,0) == Time("2000"));
+    EXPECT(Time(20,0,0) == Time("200000"));
+    EXPECT(Time(20,0,0) == Time("20", true));
+    EXPECT_THROWS(Time(30,0,0));
+    EXPECT_THROWS(Time("30"));
+    EXPECT_THROWS(Time("3000"));
+    EXPECT_THROWS(Time("300000"));
+    EXPECT(Time(30,0,0, true) == Time("30", true));
+
+
     EXPECT(Time(0,3,0) == Time("003"));
     EXPECT(Time(0,3,0) == Time("0003"));
     EXPECT(Time(0,3,0) == Time("00300"));
@@ -108,24 +133,22 @@ CASE("Time format (hh:mm:ss)") {
 CASE("Time with unit (__h__m__s)") {
     EXPECT(Time(2,0,0) == Time("2h"));
     EXPECT(Time(2,0,0) == Time("0002H"));
-    EXPECT_THROWS(Time("120m"));
-    EXPECT(Time(2,0,0) == Time("120m", true));
-    EXPECT_THROWS(Time("7200s"));
-    EXPECT(Time(2,0,0) == Time("7200s", true));
+    EXPECT(Time(2,0,0) == Time("120m"));
+    EXPECT(Time(2,0,0) == Time("7200s"));
 
     EXPECT(Time(0,3,0) == Time("3M"));
-    EXPECT(Time(0,3,0) == Time("180s", true));
+    EXPECT(Time(0,3,0) == Time("180s"));
 
     EXPECT(Time(1,23,45) == Time("1h23m45s"));
     EXPECT(Time(1,23,45) == Time("01h23m45s"));
-    EXPECT(Time(1,23,45) == Time("5025s", true));
-    EXPECT(Time(1,23,45) == Time("83m45s", true));
+    EXPECT(Time(1,23,45) == Time("5025s"));
+    EXPECT(Time(1,23,45) == Time("83m45s"));
 
     EXPECT_THROWS(Time("25h"));
     EXPECT_NO_THROW(Time("25h", true));
 
     EXPECT(Time("0d3h10m20s") == Time("3h10m20s"));
-    EXPECT(Time("0d3h10m20s") == Time("3h620s", true));
+    EXPECT(Time("0d3h10m20s") == Time("3h620s"));
     EXPECT(Time("2D3h", true) == Time("51h", true));
 }