Refactor Calculator methods to return std::optional.

The methods CalculateString, Tokenize, and CalculateTokens in Calculator are refactored to return std::optional<T> instead of using a boolean return value with an output parameter. This improves clarity by explicitly indicating whether a valid result is available. Call sites in calculator_test.cc and calculator_rewriter.cc are updated to handle the std::optional return type.

#codehealth

PiperOrigin-RevId: 912442676

Author: hiroyuki-komatsu

src/rewriter/calculator/calculator.cc

@@ -318,12 +318,11 @@ Calculator::Calculator() {
   operator_map_["abs"] = TokenType::FUNC_ABS;
 }
 
-bool Calculator::CalculateString(const absl::string_view key,
-                                 std::string* result) const {
-  DCHECK(result);
+std::optional<std::string> Calculator::CalculateString(
+    const absl::string_view key) const {
   if (key.empty() || key.size() > kMaxInputLength) {
     LOG(ERROR) << "Key is empty or too long.";
-    return false;
+    return std::nullopt;
   }
   std::string normalized_key =
       japanese_util::FullWidthAsciiToHalfWidthAscii(key);
@@ -339,37 +338,32 @@ bool Calculator::CalculateString(const absl::string_view key,
         absl::string_view(normalized_key.data(), normalized_key.size() - 1);
   } else {
     // Expression does not start nor end with '='.
-    result->clear();
-    return false;
+    return std::nullopt;
   }
 
-  TokenSequence tokens;
-  if (!Tokenize(expression_body, &tokens)) {
+  std::optional<TokenSequence> tokens = Tokenize(expression_body);
+  if (!tokens.has_value()) {
     // normalized_key is not valid sequence of tokens
-    result->clear();
-    return false;
+    return std::nullopt;
   }
 
-  double result_value = 0.0;
-  if (!CalculateTokens(tokens, &result_value)) {
+  std::optional<double> result_value = CalculateTokens(*tokens);
+  if (!result_value.has_value()) {
     // Calculation is failed. Syntax error or arithmetic error such as
     // overflow, divide-by-zero, etc.
-    result->clear();
-    return false;
+    return std::nullopt;
   }
-  *result = absl::StrFormat("%.8g", result_value);
-  return true;
+  return absl::StrFormat("%.8g", *result_value);
 }
 
-bool Calculator::Tokenize(absl::string_view expression_body,
-                          TokenSequence* tokens) const {
+std::optional<Calculator::TokenSequence> Calculator::Tokenize(
+    absl::string_view expression_body) const {
   const char* current = expression_body.data();
   const char* end = expression_body.data() + expression_body.size();
   int num_operator = 0;  // Number of operators or functions appeared
   int num_value = 0;     // Number of values appeared
 
-  DCHECK(tokens);
-  tokens->clear();
+  TokenSequence tokens;
 
   while (current < end) {
     // Skip spaces
@@ -386,9 +380,9 @@ bool Calculator::Tokenize(absl::string_view expression_body,
       std::string number_token(token_begin, current - token_begin);
       double value = 0.0;
       if (!NumberUtil::SafeStrToDouble(number_token, &value)) {
-        return false;
+        return std::nullopt;
       }
-      tokens->push_back({TokenType::INTEGER, value});
+      tokens.push_back({TokenType::INTEGER, value});
       ++num_value;
       continue;
     }
@@ -402,25 +396,25 @@ bool Calculator::Tokenize(absl::string_view expression_body,
       absl::AsciiStrToLower(&name);
       const auto func_it = operator_map_.find(name);
       if (func_it != operator_map_.end()) {
-        tokens->push_back({func_it->second, 0.0});
+        tokens.push_back({func_it->second, 0.0});
         ++num_operator;
         continue;
       }
-      return false;
+      return std::nullopt;
     }
 
     // Read operator token
     for (size_t length = 1; length <= kMaxLengthOfOperator; ++length) {
       if (current + length > end) {
         // Invalid token
-        return false;
+        return std::nullopt;
       }
       absl::string_view window(current, length);
       const auto op_it = operator_map_.find(window);
       if (op_it == operator_map_.end()) {
         continue;
       }
-      tokens->push_back({op_it->second, 0.0});
+      tokens.push_back({op_it->second, 0.0});
       current += length;
       // Does not count parenthesis as an operator.
       if ((op_it->second != TokenType::LP) &&
@@ -434,29 +428,27 @@ bool Calculator::Tokenize(absl::string_view expression_body,
     }
 
     // Invalid token
-    return false;
+    return std::nullopt;
   }
 
   if (num_operator == 0 || num_value == 0) {
     // Must contain at least one operator and one value.
-    return false;
+    return std::nullopt;
   }
-  return true;
+  return tokens;
 }
 
-bool Calculator::CalculateTokens(const TokenSequence& tokens,
-                                 double* result_value) const {
-  DCHECK(result_value);
+std::optional<double> Calculator::CalculateTokens(
+    const TokenSequence& tokens) const {
   ParserContext ctx{tokens, 0};
   std::optional<double> result = ParseExpression(ctx);
   if (!result.has_value()) {
-    return false;
+    return std::nullopt;
   }
   if (ctx.pos != ctx.tokens.size()) {
-    return false;
+    return std::nullopt;
   }
-  *result_value = *result;
-  return true;
+  return result;
 }
 
 }  // namespace mozc

src/rewriter/calculator/calculator.h

@@ -31,6 +31,7 @@
 #define MOZC_REWRITER_CALCULATOR_CALCULATOR_H_
 
 #include <cstddef>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -70,22 +71,24 @@ class Calculator {
 
   Calculator();
 
-  bool CalculateString(absl::string_view key, std::string* result) const;
+  std::optional<std::string> CalculateString(absl::string_view key) const;
 
  private:
   using TokenSequence = std::vector<Token>;
 
   // Max byte length of operator character
   static constexpr size_t kMaxLengthOfOperator = 4;
 
-  // Tokenizes |expression_body| and sets the tokens into |tokens|.
-  // It returns false if |expression_body| includes an invalid token or
+  // Tokenizes |expression_body| and returns the tokens if the tokenization
+  // is successful.
+  // It returns std::nullopt if |expression_body| includes an invalid token or
   // does not include both of a number token and an operator token.
   // Parenthesis is not considered as an operator.
-  bool Tokenize(absl::string_view expression_body, TokenSequence* tokens) const;
+  std::optional<TokenSequence> Tokenize(
+      absl::string_view expression_body) const;
 
   // Perform calculation with a given sequence of token.
-  bool CalculateTokens(const TokenSequence& tokens, double* result_value) const;
+  std::optional<double> CalculateTokens(const TokenSequence& tokens) const;
 
   // Mapping from operator character such as '+' or "log" to the corresponding
   // token type.

src/rewriter/calculator/calculator_test.cc

@@ -32,13 +32,15 @@
 #include <cmath>
 #include <cstdlib>
 #include <fstream>
+#include <optional>
 #include <ostream>
 #include <string>
 
 #include "absl/log/check.h"
 #include "absl/log/log.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/string_view.h"
+#include "testing/gmock.h"
 #include "testing/gunit.h"
 #include "testing/mozctest.h"
 
@@ -49,11 +51,14 @@ namespace {
 void VerifyCalculation(const Calculator& calculator,
                        const absl::string_view expression,
                        const absl::string_view expected) {
-  std::string result;
-  EXPECT_TRUE(calculator.CalculateString(expression, &result))
+  std::optional<std::string> result = calculator.CalculateString(expression);
+  EXPECT_TRUE(result.has_value())
       << expression << "  expected = " << expected;
+  if (!result.has_value()) {
+    return;
+  }
   double result_val;
-  EXPECT_TRUE(absl::SimpleAtod(result, &result_val));
+  EXPECT_TRUE(absl::SimpleAtod(*result, &result_val));
   double expected_val;
   EXPECT_TRUE(absl::SimpleAtod(expected, &expected_val));
   const double err = std::fabs(result_val - expected_val);
@@ -62,24 +67,22 @@ void VerifyCalculation(const Calculator& calculator,
       << "comparison: " << result_val << " vs " << expected_val << std::endl
       << "error: " << err << std::endl
       << "expr = " << expression << std::endl
-      << "result = " << result;
+      << "result = " << *result;
 }
 
 // Runs calculation and compare results in PRINTED string.
 void VerifyCalculationInString(const Calculator& calculator,
                                const absl::string_view expression,
                                const absl::string_view expected) {
-  std::string result;
-  EXPECT_TRUE(calculator.CalculateString(expression, &result))
-      << expression << "  expected = " << expected;
-  EXPECT_EQ(result, expected) << "expr = " << expression << std::endl;
+  ASSERT_THAT(calculator.CalculateString(expression),
+              ::testing::Optional(std::string(expected)))
+      << "expr = " << expression;
 }
 
 // Tries to calculate |wrong_key| and returns true if it fails.
 void VerifyRejection(const Calculator& calculator,
                      const absl::string_view wrong_key) {
-  std::string result;
-  EXPECT_FALSE(calculator.CalculateString(wrong_key, &result))
+  EXPECT_FALSE(calculator.CalculateString(wrong_key).has_value())
       << "expression: " << wrong_key << std::endl;
 }
 
@@ -191,8 +194,8 @@ TEST(CalculatorTest, StressTest) {
     // If (__ANDROID__ && x86) the result differs from expectation
     // because of floating point specification so on such environment
     // Following verification is skipped.
-    std::string unused_result;
-    calculator.CalculateString(query, &unused_result);
+    std::optional<std::string> unused_result =
+        calculator.CalculateString(query);
 #if !defined(__ANDROID__) || !defined(__i386__)
     if (line.size() == query_length) {
       // False test
@@ -208,15 +211,14 @@ TEST(CalculatorTest, StressTest) {
 
 TEST(CalculatorTest, LimitAndValidationTest) {
   Calculator calculator;
-  std::string unused_result;
 
   // Large but valid operations under limit (e.g. 100 unary operations).
   std::string valid_case = std::string(100, '+') + "1=";
-  EXPECT_TRUE(calculator.CalculateString(valid_case, &unused_result));
+  EXPECT_TRUE(calculator.CalculateString(valid_case).has_value());
 
   // Directly violating input length limit (e.g. 2100 bytes).
   std::string over_length_case = std::string(2100, '+') + "1=";
-  EXPECT_FALSE(calculator.CalculateString(over_length_case, &unused_result));
+  EXPECT_FALSE(calculator.CalculateString(over_length_case).has_value());
 }
 
 }  // namespace mozc

src/rewriter/calculator_rewriter.cc

@@ -79,8 +79,8 @@ CalculatorRewriter::CheckResizeSegmentsRequest(const ConversionRequest& request,
   }
   // The decision to calculate and calculation itself are both done by the
   // calculator.
-  std::string result;
-  if (!calculator_.CalculateString(merged_key, &result)) {
+  std::optional<std::string> result = calculator_.CalculateString(merged_key);
+  if (!result.has_value()) {
     return std::nullopt;
   }
 
@@ -113,13 +113,13 @@ bool CalculatorRewriter::Rewrite(const ConversionRequest& request,
     return false;
   }
 
-  std::string result;
-  if (!calculator_.CalculateString(key, &result)) {
+  std::optional<std::string> result = calculator_.CalculateString(key);
+  if (!result.has_value()) {
     return false;
   }
 
   // Insert the result.
-  if (!InsertCandidate(result, 0, segments->mutable_conversion_segment(0))) {
+  if (!InsertCandidate(*result, 0, segments->mutable_conversion_segment(0))) {
     return false;
   }