simplify code, reduce duplication

1 files changed, 37 insertions, 66 deletions

diff --git a/src/displayapp/screens/Calculator.cpp b/src/displayapp/screens/Calculator.cpp
index 42688a3..dd23059 100644
--- a/src/displayapp/screens/Calculator.cpp
+++ b/src/displayapp/screens/Calculator.cpp
@@ -7,7 +7,6 @@
 
 using namespace Pinetime::Applications::Screens;
 
-// Anonymous Namespace for all the structs
 namespace {
   template <typename X, uint8_t max_stack_len> struct Stack {
     // Basic stack data type without dynamic allocations
@@ -21,58 +20,40 @@ namespace {
     inline X&  push()   { return data[stack_len++];        }
     inline void push(X x) { X& datum{ push() }; datum = x; }
   };
-  struct Node {
-    // if op == 0, the node represents a double value, otherwise it's an operator
-    char op;
-    double val;
-  };
-  template <uint8_t max_stack_len> struct CalcStack : public Stack<Node, max_stack_len> {
-    typedef Stack<Node, max_stack_len> Super;
-    inline Node& push() { return Super::push(); }
+  template <uint8_t max_stack_len> struct CalcStack : public Stack<double, max_stack_len> {
+    typedef Stack<double, max_stack_len> Super;
     inline void pushValue(double value) {
-      Super::data[Super::stack_len].op = 0;
-      Super::data[Super::stack_len].val = value;
-      Super::stack_len++;
+      Super::push(value);
     }
-    void pushOperator(char op) {
-      Node* node0 = Super::data + Super::stack_len;
-      node0->op = op;
-      if (Super::stack_len > 1) {
-        // Evaluate subexpressions as soon as it's possible
-        // Inf and NaN take care of error handling in case of non-finite results
-        Node* node1 = node0 - 1;
-        if (node1->op == 0) {
-          Node* node2 = node1 - 1;
-          auto val2 = node2->val;
-          auto val1 = node1->val;
-          if (node2->op == 0) {
-            switch (op) {
-            case '^':
-              val2 = pow(val2, val1);
-              break;
-            case 'x':
-              val2 *= val1;
-              break;
-            case '/':
-              val2 /= val1;
-              break;
-            case '+':
-              val2 += val1;
-              break;
-            case '-':
-              val2 -= val1;
-              break;
-            default:
-              goto done;
-            }
-          }
-          node2->val = val2;
-          Super::stack_len--;
-          return;
-        } 
+    bool pushOperator(char op) {
+      if (Super::size() < 2) return false;
+      double* node0 = Super::data + Super::stack_len;
+      // Call this only when stack_len >= 2
+      // Evaluate subexpressions as soon as possible
+      // Inf and NaN take care of error handling in case of non-finite results
+      auto& val2 { node0[-2] };
+      auto val1 = node0[-1];
+      switch (op) {
+      case '^':
+        val2 = pow(val2, val1);
+        break;
+      case 'x':
+        val2 *= val1;
+        break;
+      case '/':
+        val2 /= val1;
+        break;
+      case '+':
+        val2 += val1;
+        break;
+      case '-':
+        val2 -= val1;
+        break;
+      default:
+        return false;
       }
-     done:
-      Super::stack_len++;
+      Super::stack_len--;
+      return true;
     }
   };
 
@@ -190,8 +171,7 @@ bool Calculator::Eval() {
     }
 
     if (isdigit(input.top())) {
-      if (!parseFloat(input, output.push().val, sign)) return false;
-      output.top().op = 0;
+      if (!parseFloat(input, output.push(), sign)) return false;
       sign = +1;
       expectingNumber = false;
       continue;
@@ -226,8 +206,7 @@ bool Calculator::Eval() {
                    // or (the operator at the top of the operator stack has equal precedence and the token is left associative))
                    || (getPrecedence(operators.top()) == getPrecedence(next) && leftAssociative(next)))) {
           // need two elements on the output stack to add a binary operator
-          if (output.size() < 2) return false;
-          output.pushOperator(operators.top());
+          if (!output.pushOperator(operators.top())) return false;
           operators.pop();
         }
         operators.push(next);
@@ -253,8 +232,7 @@ bool Calculator::Eval() {
       case ')':
         while (operators.top() != '(') {
           // need two elements on the output stack to add a binary operator
-          if (output.size() < 2) return false;
-          output.pushOperator(operators.top());
+          if (!output.pushOperator(operators.top())) return false;
           operators.pop();
           if (operators.empty()) return false;
         }
@@ -263,18 +241,11 @@ bool Calculator::Eval() {
     }
   }
   while (!operators.empty()) {
-    char op = operators.pop();
-    if ((op == ')' || op == '(')
-        ||
-        // need two elements on the output stack to add a binary operator
-        (output.size() < 2))
-    {
-      return false;
-    }
-    output.pushOperator(op);
+    output.pushOperator(operators.pop());
   }
   // perform the calculation
-  resultFloat = output.top().val;
+  // assert(output.size() == 1);
+  resultFloat = output.data[0]; // this is the same as output.top() when output.size() == 1, but may reduce binary size
   // make sure that the absolute value of the integral part of result fits in a 32 bit unsigned integer
   sign = (resultFloat < 0);
   if (sign) {