telegramparser: return std::optional<std::string> instead of passing it by ref argument

w0lek · w0lek · commit 49dd7ab7464e · 2024-04-16T13:29:21.000+03:00
diff --git a/vpr/src/server/gateio.cpp b/vpr/src/server/gateio.cpp
@@ -157,8 +157,7 @@ GateIO::ActivityStatus GateIO::handleTelegrams(std::vector<comm::TelegramFramePt
             m_logger.queue(LogLevel::Detail, "received composed", getPrettySizeStrFromBytesNum(message.size()), ":", getTruncatedMiddleStr(message));
             std::optional<int> jobIdOpt = comm::TelegramParser::tryExtractFieldJobId(message);
             std::optional<int> cmdOpt = comm::TelegramParser::tryExtractFieldCmd(message);
-            std::optional<std::string> optionsOpt;
-            comm::TelegramParser::tryExtractFieldOptions(message, optionsOpt);
+            std::optional<std::string> optionsOpt = comm::TelegramParser::tryExtractFieldOptions(message);
             if (jobIdOpt && cmdOpt && optionsOpt) {
                 TaskPtr task = std::make_unique<Task>(jobIdOpt.value(), cmdOpt.value(), optionsOpt.value());
                 const comm::TelegramHeader& header = telegramFrame->header;
diff --git a/vpr/src/server/telegramparser.cpp b/vpr/src/server/telegramparser.cpp
@@ -5,75 +5,68 @@
 
 namespace comm {
 
-bool TelegramParser::tryExtractJsonValueStr(const std::string& jsonString, const std::string& key, std::optional<std::string>& result)
+std::optional<std::string> TelegramParser::tryExtractJsonValueStr(const std::string& jsonString, const std::string& key)
 {
     // Find the position of the key
     size_t keyPos = jsonString.find("\"" + key + "\":");
 
     if (keyPos == std::string::npos) {
         // Key not found
-        return false;
+        return std::nullopt;
     }
 
     // Find the position of the value after the key
     size_t valuePosStart = jsonString.find("\"", keyPos + key.length() + std::string("\":\"").size());
 
     if (valuePosStart == std::string::npos) {
         // Value not found
-        return false;
+        return std::nullopt;
     }
 
     // Find the position of the closing quote for the value
     size_t valueEnd = jsonString.find("\"", valuePosStart + std::string("\"").size());
 
     if (valueEnd == std::string::npos) {
         // Closing quote not found
-        return false;
+        return std::nullopt;
     }
 
     // Extract the value substring
-    result = jsonString.substr(valuePosStart + 1, (valueEnd - valuePosStart) - 1);
-    return true;
+    return jsonString.substr(valuePosStart + 1, (valueEnd - valuePosStart) - 1);
 }
 
 std::optional<int> TelegramParser::tryExtractFieldJobId(const std::string& message)
 {
-    std::optional<int> result;
-    std::optional<std::string> strOpt;
-    if (tryExtractJsonValueStr(message, comm::KEY_JOB_ID, strOpt)) {
-        result = tryConvertToInt(strOpt.value());
+    if (std::optional<std::string> strOpt = tryExtractJsonValueStr(message, comm::KEY_JOB_ID)) {
+        return tryConvertToInt(strOpt.value());
     }
-    return result;
+    return std::nullopt;
 }
 
 std::optional<int> TelegramParser::tryExtractFieldCmd(const std::string& message)
 {
-    std::optional<int> result;
-    std::optional<std::string> strOpt;
-    if (tryExtractJsonValueStr(message, comm::KEY_CMD, strOpt)) {
-        result = tryConvertToInt(strOpt.value());
+    if (std::optional<std::string> strOpt = tryExtractJsonValueStr(message, comm::KEY_CMD)) {
+        return tryConvertToInt(strOpt.value());
     }
-    return result;
+    return std::nullopt;
 }
 
-bool TelegramParser::tryExtractFieldOptions(const std::string& message, std::optional<std::string>& result)
+std::optional<std::string> TelegramParser::tryExtractFieldOptions(const std::string& message)
 {
-    return tryExtractJsonValueStr(message, comm::KEY_OPTIONS, result);
+    return tryExtractJsonValueStr(message, comm::KEY_OPTIONS);
 }
 
-bool TelegramParser::tryExtractFieldData(const std::string& message, std::optional<std::string>& result)
+std::optional<std::string> TelegramParser::tryExtractFieldData(const std::string& message)
 {
-    return tryExtractJsonValueStr(message, comm::KEY_DATA, result);
+    return tryExtractJsonValueStr(message, comm::KEY_DATA);
 }
 
 std::optional<int> TelegramParser::tryExtractFieldStatus(const std::string& message)
 {
-    std::optional<int> result;
-    std::optional<std::string> strOpt;
-    if (tryExtractJsonValueStr(message, comm::KEY_STATUS, strOpt)) {
-        result = tryConvertToInt(strOpt.value());
+    if (std::optional<std::string> strOpt = tryExtractJsonValueStr(message, comm::KEY_STATUS)) {
+        return tryConvertToInt(strOpt.value());
     }
-    return result;
+    return std::nullopt;
 }
 
 } // namespace comm
diff --git a/vpr/src/server/telegramparser.h b/vpr/src/server/telegramparser.h
@@ -16,12 +16,12 @@ class TelegramParser {
 public:
     static std::optional<int> tryExtractFieldJobId(const std::string& message);
     static std::optional<int> tryExtractFieldCmd(const std::string& message);
-    static bool tryExtractFieldOptions(const std::string& message, std::optional<std::string>& result);
-    static bool tryExtractFieldData(const std::string& message, std::optional<std::string>& result);
+    static std::optional<std::string> tryExtractFieldOptions(const std::string& message);
+    static std::optional<std::string> tryExtractFieldData(const std::string& message);
     static std::optional<int> tryExtractFieldStatus(const std::string& message);
 
 private:
-    static bool tryExtractJsonValueStr(const std::string& jsonString, const std::string& key, std::optional<std::string>& result);
+    static std::optional<std::string> tryExtractJsonValueStr(const std::string& jsonString, const std::string& key);
 };
 
 } // namespace comm
diff --git a/vpr/test/test_server_telegramparser.cpp b/vpr/test/test_server_telegramparser.cpp
@@ -9,12 +9,8 @@ TEST_CASE("test_server_telegram_parser_base", "[vpr]")
 
     REQUIRE(std::optional<int>{7} == comm::TelegramParser::tryExtractFieldJobId(tdata));
     REQUIRE(std::optional<int>{2} == comm::TelegramParser::tryExtractFieldCmd(tdata));
-    std::optional<std::string> optionsOpt;
-    REQUIRE(comm::TelegramParser::tryExtractFieldOptions(tdata, optionsOpt) == true);
-    REQUIRE(std::optional<std::string>{"type1:name1:value1;type2:name2:v a l u e 2;t3:n3:v3"} == optionsOpt);
-    std::optional<std::string> dataOpt;
-    REQUIRE(comm::TelegramParser::tryExtractFieldData(tdata, dataOpt) == true);
-    REQUIRE(std::optional<std::string>{"some_data..."} == dataOpt);
+    REQUIRE(std::optional<std::string>{"type1:name1:value1;type2:name2:v a l u e 2;t3:n3:v3"} == comm::TelegramParser::tryExtractFieldOptions(tdata));
+    REQUIRE(std::optional<std::string>{"some_data..."} == comm::TelegramParser::tryExtractFieldData(tdata));
     REQUIRE(std::optional<int>{1} == comm::TelegramParser::tryExtractFieldStatus(tdata));
 }
 
@@ -24,12 +20,8 @@ TEST_CASE("test_server_telegram_parser_invalid_keys", "[vpr]")
     
     REQUIRE(std::nullopt == comm::TelegramParser::tryExtractFieldJobId(tBadData));
     REQUIRE(std::nullopt == comm::TelegramParser::tryExtractFieldCmd(tBadData));
-    std::optional<std::string> optionsOpt;
-    REQUIRE_FALSE(comm::TelegramParser::tryExtractFieldOptions(tBadData, optionsOpt));
-    REQUIRE(std::nullopt == optionsOpt);
-    std::optional<std::string> dataOpt;
-    REQUIRE_FALSE(comm::TelegramParser::tryExtractFieldData(tBadData, dataOpt));
-    REQUIRE(std::nullopt == dataOpt);
+    REQUIRE(std::nullopt == comm::TelegramParser::tryExtractFieldOptions(tBadData));
+    REQUIRE(std::nullopt == comm::TelegramParser::tryExtractFieldData(tBadData));
     REQUIRE(std::nullopt == comm::TelegramParser::tryExtractFieldStatus(tBadData));
 }
 

Original file line number	Diff line number	Diff line change
`@@ -5,75 +5,68 @@`
`5`	`5`
`6`	`6`	`namespace comm {`
`7`	`7`
`8`		`-bool TelegramParser::tryExtractJsonValueStr(const std::string& jsonString, const std::string& key, std::optional<std::string>& result)`
	`8`	`+std::optional<std::string> TelegramParser::tryExtractJsonValueStr(const std::string& jsonString, const std::string& key)`
`9`	`9`	`{`
`10`	`10`	`// Find the position of the key`
`11`	`11`	`size_t keyPos = jsonString.find("\"" + key + "\":");`
`12`	`12`
`13`	`13`	`if (keyPos == std::string::npos) {`
`14`	`14`	`// Key not found`
`15`		`- return false;`
	`15`	`+ return std::nullopt;`
`16`	`16`	`}`
`17`	`17`
`18`	`18`	`// Find the position of the value after the key`
`19`	`19`	`size_t valuePosStart = jsonString.find("\"", keyPos + key.length() + std::string("\":\"").size());`
`20`	`20`
`21`	`21`	`if (valuePosStart == std::string::npos) {`
`22`	`22`	`// Value not found`
`23`		`- return false;`
	`23`	`+ return std::nullopt;`
`24`	`24`	`}`
`25`	`25`
`26`	`26`	`// Find the position of the closing quote for the value`
`27`	`27`	`size_t valueEnd = jsonString.find("\"", valuePosStart + std::string("\"").size());`
`28`	`28`
`29`	`29`	`if (valueEnd == std::string::npos) {`
`30`	`30`	`// Closing quote not found`
`31`		`- return false;`
	`31`	`+ return std::nullopt;`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`// Extract the value substring`
`35`		`- result = jsonString.substr(valuePosStart + 1, (valueEnd - valuePosStart) - 1);`
`36`		`- return true;`
	`35`	`+ return jsonString.substr(valuePosStart + 1, (valueEnd - valuePosStart) - 1);`
`37`	`36`	`}`
`38`	`37`
`39`	`38`	`std::optional<int> TelegramParser::tryExtractFieldJobId(const std::string& message)`
`40`	`39`	`{`
`41`		`- std::optional<int> result;`
`42`		`- std::optional<std::string> strOpt;`
`43`		`- if (tryExtractJsonValueStr(message, comm::KEY_JOB_ID, strOpt)) {`
`44`		`- result = tryConvertToInt(strOpt.value());`
	`40`	`+ if (std::optional<std::string> strOpt = tryExtractJsonValueStr(message, comm::KEY_JOB_ID)) {`
	`41`	`+ return tryConvertToInt(strOpt.value());`
`45`	`42`	`}`
`46`		`- return result;`
	`43`	`+ return std::nullopt;`
`47`	`44`	`}`
`48`	`45`
`49`	`46`	`std::optional<int> TelegramParser::tryExtractFieldCmd(const std::string& message)`
`50`	`47`	`{`
`51`		`- std::optional<int> result;`
`52`		`- std::optional<std::string> strOpt;`
`53`		`- if (tryExtractJsonValueStr(message, comm::KEY_CMD, strOpt)) {`
`54`		`- result = tryConvertToInt(strOpt.value());`
	`48`	`+ if (std::optional<std::string> strOpt = tryExtractJsonValueStr(message, comm::KEY_CMD)) {`
	`49`	`+ return tryConvertToInt(strOpt.value());`
`55`	`50`	`}`
`56`		`- return result;`
	`51`	`+ return std::nullopt;`
`57`	`52`	`}`
`58`	`53`
`59`		`-bool TelegramParser::tryExtractFieldOptions(const std::string& message, std::optional<std::string>& result)`
	`54`	`+std::optional<std::string> TelegramParser::tryExtractFieldOptions(const std::string& message)`
`60`	`55`	`{`
`61`		`- return tryExtractJsonValueStr(message, comm::KEY_OPTIONS, result);`
	`56`	`+ return tryExtractJsonValueStr(message, comm::KEY_OPTIONS);`
`62`	`57`	`}`
`63`	`58`
`64`		`-bool TelegramParser::tryExtractFieldData(const std::string& message, std::optional<std::string>& result)`
	`59`	`+std::optional<std::string> TelegramParser::tryExtractFieldData(const std::string& message)`
`65`	`60`	`{`
`66`		`- return tryExtractJsonValueStr(message, comm::KEY_DATA, result);`
	`61`	`+ return tryExtractJsonValueStr(message, comm::KEY_DATA);`
`67`	`62`	`}`
`68`	`63`
`69`	`64`	`std::optional<int> TelegramParser::tryExtractFieldStatus(const std::string& message)`
`70`	`65`	`{`
`71`		`- std::optional<int> result;`
`72`		`- std::optional<std::string> strOpt;`
`73`		`- if (tryExtractJsonValueStr(message, comm::KEY_STATUS, strOpt)) {`
`74`		`- result = tryConvertToInt(strOpt.value());`
	`66`	`+ if (std::optional<std::string> strOpt = tryExtractJsonValueStr(message, comm::KEY_STATUS)) {`
	`67`	`+ return tryConvertToInt(strOpt.value());`
`75`	`68`	`}`
`76`		`- return result;`
	`69`	`+ return std::nullopt;`
`77`	`70`	`}`
`78`	`71`
`79`	`72`	`} // namespace comm`