Reducing Function Complexity

Question

I'm writing a simple calculator in React. I'm trying to update the state of a component based on which button is clicked (as you would expect). The component's state is as follows:

this.state = {
  total: null,
  next: null,
  operation: null,
}

Presently, I have one long if/else statement that checks a bunch of scenarios to determine what the appropriate values should be. I'm wondering how I could rewrite this to reduce the complexity of the function:

handleClick(buttonName) {
    let { total, next } = this.state;
    const { operation } = this.state;
    let obj;
    const nums = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
    const syms = ['+', '-', 'X', '/', '%'];
    const equal = '=';
    const reset = 'AC';
    const decimalPoint = '.';

    if (total === null && nums.indexOf(buttonName) >= 0) {
      this.setState({
        total: buttonName,
      });
    } else if (total !== null
      && buttonName === decimalPoint
      && (total.indexOf(decimalPoint) === -1)) {
      this.setState({
        total: total += buttonName,
      });
    } else if (total !== null
      && next !== null
      && buttonName === decimalPoint
      && (next.indexOf(decimalPoint) === -1)) {
      this.setState({
        next: next += buttonName,
      });
    } else if (total !== null
      && operation === null
      && syms.indexOf(buttonName) >= 0) {
      this.setState({
        operation: buttonName,
      });
    } else if (total !== null
      && operation === null
      && nums.indexOf(buttonName) >= 0) {
      this.setState({
        total: total += buttonName,
      });
    } else if (total !== null
      && operation !== null
      && next === null
      && nums.indexOf(buttonName) >= 0) {
      this.setState({
        next: buttonName,
      });
    } else if (total !== null
      && operation !== null
      && next !== null
      && nums.indexOf(buttonName) >= 0) {
      this.setState({
        next: next += buttonName,
      });
    } else if (total !== null
      && operation !== null
      && next !== null
      && ((buttonName === equal) || (buttonName === reset))) {
      obj = method.calculate(this.state, operation);
      this.setState({
        total: obj.total,
        next: obj.next,
        operation: buttonName,
      });
    }
  }

PS. the "method.calculate" function is stated in another module.

Simple, have different handlers for different classes of action (number entry, operation, equals etc). In any case your question might be better on [codereview](https://codereview.stackexchange.com/) — Jamiec, Feb 17 '20 at 15:04
Didn't know codereview was a thing, tbh. I'll post it there as well, thanks. — Genetic1989, Feb 17 '20 at 15:25

score 0 · Answer 1 · answered Feb 17 '20 at 15:33

In short:

Try to store your conditions values in variables (please try to give them better names)

   const check1 = nums.indexOf(buttonName) >= 0
   const check2 = buttonName === decimalPoint
   const check3 = next.indexOf(decimalPoint) === -1)

Then, try to reconstruct your statements as nested, using the variables instead of expressions. For instance:

    if (total) {
     if (check2 && check3) {
        if (next) {
           this.setState({ next: next += buttonName });
        }
        else {
           this.setState({total: total += buttonName });
        }
     }
     else {
     }
    }

This is of-course just a proposal. In your case it will:

Stop re-calculating expressions over and over again
Make code more readable and maintainable
Allow you to collaborate with other developers

And yes.. Stack Exchange Code review is a thing :)

Ben Aston · Accepted Answer · 2020-02-17T18:06:41.823

You can reify the click handler conditions, and then have a utility function select and invoke the relevant handler.

I don't know if this will work in your case because I am not sure if the reference to state is constant and whether setState is target-dependent, but this serves as an example.

Edit: I have investigated how React works, and the precise code below will likely not work, but the general approach is sound.

const createHandler = 
    ({ state, setState }) => 
        (buttonName) => handlers.some(({condition, action}) => 
            condition({ buttonName, state: this.state}) && 
            action({buttonName, state, setState}) && 
            true)

const handlers = [ 
    { 
        condition: ({ state: { total }}) => {
            const nums = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']
            return (total === null && nums.indexOf(buttonName) >= 0)
        },
        action: ({ buttonName, setState }) => setState({ total: buttonName })
    },
    // ...
];

class MyComponent {
    constructor() {
        this.handle = createHandler({ state: this.state, setState: this.setState })
    }

    handleClick(buttonName) {
        this.handle(buttonName)
    }
}

Reducing Function Complexity

2 Answers2