Calculator Not Following Order of Operations (PEMDAS/BODMAS)

[muhammadnoorulainroy]

The calculator component processes mathematical operations sequentially from left to right instead of following the standard mathematical order of operations (PEMDAS/BODMAS). This results in incorrect calculations when multiple operators with different precedence levels are used in a single expression.

---

Environment

• File: src/pages/activities/Calculator.js
• Component Type: React Class Component
• Lines of Interest: 17-74 (calculation logic in screenUpdate method)
• Browser: All browsers (logic error, not browser-specific)
• Version: Current main branch

---

Current Behavior

The calculator evaluates expressions sequentially from left to right, processing each operator as it is encountered regardless of operator precedence. This violates fundamental mathematical principles and produces incorrect results.

Example 1: Multiplication Before Addition

Input: 2 + 3 × 4

• Current Output: 20
• Expected Output: 14
• Calculation Path (Current): (2 + 3) × 4 = 5 × 4 = 20
• Calculation Path (Expected): 2 + (3 × 4) = 2 + 12 = 14

Example 2: Division Before Subtraction

Input: 10 - 8 / 2

• Current Output: 1
• Expected Output: 6
• Calculation Path (Current): (10 - 8) / 2 = 2 / 2 = 1
• Calculation Path (Expected): 10 - (8 / 2) = 10 - 4 = 6

Example 3: Multiple Operations

Input: 2 + 3 × 4 - 5

• Current Output: 15
• Expected Output: 9
• Calculation Path (Current): ((2 + 3) × 4) - 5 = (5 × 4) - 5 = 20 - 5 = 15
• Calculation Path (Expected): 2 + (3 × 4) - 5 = 2 + 12 - 5 = 9

---

Expected Behavior

The calculator should follow the standard mathematical order of operations (PEMDAS/BODMAS):

1. Parentheses / Brackets
2. Exponents / Orders
3. Multiplication and Division (left to right, equal precedence)
4. Addition and Subtraction (left to right, equal precedence)

When evaluating 2 + 3 × 4:

1. Identify all operators and their precedence
2. Process multiplication first: 3 × 4 = 12
3. Then process addition: 2 + 12 = 14
4. Return result: 14

---

Steps to Reproduce

1. Navigate to the Calculator activity:

   • Go to http://localhost:3000/activities/calculator (development)
   • Or visit https://acm-fun.vercel.app/activities/calculator (production)

2. Enter the following expression:

   • Click 2
   • Click +
   • Click 3
   • Click x (multiply)
   • Click 4
   • Click =

3. Observe the result displays 20 instead of 14

4. Try additional test cases:

   • 10 - 8 / 2 → Shows 1, should show 6
   • 5 × 2 + 3 → Shows 13, should show 13 (correct by coincidence)
   • 5 + 2 × 3 → Shows 21, should show 11
   • 12 / 3 - 1 → Shows 3, should show 3 (correct by coincidence)
   • 12 - 3 / 3 → Shows 3, should show 11

---

Technical Analysis

Root Cause

The issue is located in the screenUpdate method, specifically in the calculation logic (lines 17-74). The current implementation:

1. Parses the input string into an array of numbers (arr) and operators (op)
2. Iterates through operators sequentially
3. Applies each operation immediately to the first two numbers in the array
4. Shifts the result back into the array

Problematic Code Section (lines 56-62):

for(let b = 0; b < op.length; b++) {
    if(op[b] === '/') {r = arr[0] / arr[1]; arr.shift(); arr[0] = r}
    if(op[b] === 'x') {r = arr[0] * arr[1]; arr.shift(); arr[0] = r}
    if(op[b] === '-') {r = arr[0] - arr[1]; arr.shift(); arr[0] = r}
    if(op[b] === '+') {r = arr[0] + arr[1]; arr.shift(); arr[0] = r}
    console.log(r)
}

This loop processes operations in the order they appear in the expression, not by their mathematical precedence.

Example Execution Trace

For input 2 + 3 × 4:

Current Implementation:

Initial: arr = [2, 3, 4], op = ['+', 'x']

Iteration 1 (b=0, op[0]='+'):
  - r = arr[0] + arr[1] = 2 + 3 = 5
  - arr.shift() → arr = [3, 4]
  - arr[0] = 5 → arr = [5, 4]

Iteration 2 (b=1, op[1]='x'):
  - r = arr[0] * arr[1] = 5 * 4 = 20
  - arr.shift() → arr = [4]
  - arr[0] = 20 → arr = [20]

Result: 20 (INCORRECT)

Expected Implementation:

Initial: arr = [2, 3, 4], op = ['+', 'x']

Step 1: Identify highest precedence operators (× and /)
  - Found 'x' at index 1

Step 2: Process multiplication first
  - r = arr[1] * arr[2] = 3 * 4 = 12
  - arr = [2, 12]
  - op = ['+']

Step 3: Process addition
  - r = arr[0] + arr[1] = 2 + 12 = 14
  - arr = [14]

Result: 14 (CORRECT)

---

Proposed Solution

Approach 1: Two-Pass Algorithm (Recommended)

Implement operator precedence by processing high-precedence operators first:

// First pass: Process multiplication and division (left to right)
for(let b = 0; b < op.length; b++) {
    if(op[b] === '/' || op[b] === 'x') {
        if(op[b] === '/') {
            r = arr[b] / arr[b+1];
        } else {
            r = arr[b] * arr[b+1];
        }
        // Replace the two operands with the result
        arr.splice(b, 2, r);
        // Remove the operator
        op.splice(b, 1);
        // Adjust index since we removed an element
        b--;
    }
}

// Second pass: Process addition and subtraction (left to right)
for(let b = 0; b < op.length; b++) {
    if(op[b] === '-') {
        r = arr[b] - arr[b+1];
    } else if(op[b] === '+') {
        r = arr[b] + arr[b+1];
    }
    arr.splice(b, 2, r);
    op.splice(b, 1);
    b--;
}

Approach 2: Shunting Yard Algorithm

For a more robust solution that can handle parentheses and future operators:

1. Convert infix notation to postfix (Reverse Polish Notation) using Shunting Yard algorithm
2. Evaluate postfix expression using a stack

This approach is more scalable if parentheses or additional operators are added later.

Approach 3: Expression Parser

Use a recursive descent parser or expression tree to properly handle operator precedence. This is the most flexible approach for future enhancements.

---

Impact Assessment

Severity: High

User Impact:

• All users performing multi-operation calculations receive incorrect results
• Undermines trust in the calculator functionality
• May lead to real-world calculation errors if users rely on the tool

Functional Impact:

• Core functionality broken
• Violates mathematical standards
• Produces incorrect results for approximately 50% of multi-operation expressions

Frequency:

• Occurs every time multiple operators with different precedence are used
• Affects any expression mixing addition/subtraction with multiplication/division

Affected Use Cases

1. Basic arithmetic with mixed operations
2. Educational use (students learning order of operations would get wrong answers)
3. Quick calculations for daily tasks
4. Any mathematical expression with more than one operator type

---

Testing Requirements

Unit Tests Required

describe('Calculator Order of Operations', () => {
    test('should multiply before adding: 2 + 3 × 4', () => {
        expect(calculate('2+3x4')).toBe(14);
    });

    test('should divide before subtracting: 10 - 8 / 2', () => {
        expect(calculate('10-8/2')).toBe(6);
    });

    test('should process left to right for same precedence: 10 - 5 - 2', () => {
        expect(calculate('10-5-2')).toBe(3);
    });

    test('should handle multiple operations: 2 + 3 × 4 - 5', () => {
        expect(calculate('2+3x4-5')).toBe(9);
    });

    test('should handle division and multiplication: 12 / 3 × 2', () => {
        expect(calculate('12/3x2')).toBe(8);
    });

    test('should handle complex expressions: 5 + 2 × 3 - 4 / 2', () => {
        expect(calculate('5+2x3-4/2')).toBe(9);
    });
});

---

Additional Considerations

Edge Cases to Handle

1. Negative numbers: -5 + 3 × 2
2. Decimal operations: 2.5 + 3.5 × 2
3. Division by zero: 5 / 0 (currently not handled)
4. Very long expressions: Performance considerations
5. Consecutive operators: Should be prevented or handled

Future Enhancements

Once order of operations is fixed, consider:

1. Adding parentheses support: (2 + 3) × 4
2. Adding exponent operator: 2 ^ 3
3. Adding percentage operator: 100 + 10%
4. Adding memory functions: M+, M-, MR, MC
5. Scientific calculator mode with advanced functions

---

Related Issues

• Issue Add footer #3: Console.log statements in Calculator (should be removed)
• Issue Create sudoku #11: Refactor Calculator to functional component (technical debt)

---

References

Mathematical Standards

• Order of Operations - Wikipedia
• PEMDAS - Math is Fun

Implementation Resources

• Shunting Yard Algorithm
• Expression Parsing in JavaScript
• Math.js - Expression Parser

Similar Implementations

• Calculator.js - NPM Package
• Math Expression Evaluator

---

Acceptance Criteria

• All test cases in the testing requirements table pass
• Unit tests achieve 100% coverage for calculation logic
• Manual testing confirms correct behavior for all scenarios
• Code follows existing project structure and conventions
• Documentation updated to explain order of operations
• Console.log statements removed from calculation logic
• Edge cases handled appropriately with user feedback
• No regression in existing passing test cases

---

Implementation Checklist

• Review current implementation and identify all affected code
• Choose implementation approach (Two-Pass recommended)
• Implement operator precedence logic
• Add comprehensive unit tests
• Test with all manual test cases
• Handle edge cases (negative numbers, decimals, etc.)
• Remove debug console.log statements
• Update code comments and documentation
• Perform code review
• Test on multiple browsers
• Update user documentation if needed

---

Estimated Effort

• Complexity: Medium
• Estimated Time: 6-8 hours
• Breakdown:
  • Analysis and planning: 1 hour
  • Implementation: 3-4 hours
  • Testing: 2 hours
  • Documentation and code review: 1 hour

---

Notes for Implementer

1. The Calculator uses 'x' instead of '*' for multiplication (line 169)
2. Current implementation uses character codes for operators, maintain consistency
3. Be careful with the array manipulation - the current code modifies arrays in place
4. Consider edge cases with the existing decimal point handling logic
5. The calculator has a 15-digit limit (line 68-70) - ensure this still works
6. Preserve existing features like "Digit Limit Met!" message
7. The negative number handling (lines 31-35, 90-95) needs to work with new logic

---

[muhammadnoorulainroy]

Please assign this issue to me.