Javascript calculator stuck on tests 8,11,13

Tell us what’s happening:
Describe your issue in detail here.
I have the Javascript Calculator partially working, but tests 14-16 fail. I need some help on what to do and where in my code to make these tests pass.

Your code so far

const buttons = [
  {
    id: 'back',
    code: 8,
    name: 'C',
    shift: false
  },
  {
    id: 'clear',
    code: 8,
    name: 'AC',
    shift: true
  },
  {
    id: 'zero',
    code: 48,
    name: 0,
    shift: false
  },
  {
    id: 'one',
    code: 49,
    name: 1,
    shift: false
  },
  {
    id: 'two',
    code: 50,
    name: 2,
    shift: false
  },
  {
    id: 'three',
    code: 51,
    name: 3,
    shift: false
  },
  {
    id: 'four',
    code: 52,
    name: 4,
    shift: false
  },
  {
    id: 'five',
    code: 53,
    name: 5,
    shift: false
  },
  {
    id: 'six',
    code: 54,
    name: 6,
    shift: false
  },
  {
    id: 'seven',
    code: 55,
    name: 7,
    shift: false
  },
  {
    id: 'eight',
    code: 56,
    name: 8,
    shift: false
  },
  {
    id: 'nine',
    code: 57,
    name: 9,
    shift: false
  },
  {
    id: 'multiply',
    code: 56,
    name: '*',
    shift: true
  },
  {
    id: 'subtract',
    code: 187,
    name: '-',
    shift: true
  },
  {
    id: 'equals',
    code: 189,
    name: '=',
    shift: false
  },
  {
    id: 'add',
    code: 189,
    name: '+',
    shift: true
  },
  {
    id: 'decimal',
    code: 190,
    name: '.',
    shift: false
  },
  {
    id: 'divide',
    code: 191,
    name: '/',
    shift: false
  },
]

function App() {

  const [expression, setExpression] = React.useState("");
  const [result, setResult] = React.useState(0);

  return (
    <div id="container" className="container">
      <div id="screen" className="display">
        <input id="expression" className="expression" type="text" value={expression} placeholder="0" disabled></input>
        <input id="display" className="result" type="text" value={result} placeholder ="0" disabled></input>
      </div>
      <div id="grid" className="grid">
        {buttons.map((button) => (
          <Pad
            key={button.id}
            button={button}
            expression={expression}
            setExpression={setExpression}
            result={result}
            setResult={setResult}
          />
        ))}
      </div>
    </div>
  )
}

function Pad({ button, expression, setExpression, result, setResult }) {

  const style = {
    gridArea: button.id,
  }

  const display = () => {
    setExpression(prev => prev + button.name);
    if (expression[expression.length - 1] == "=") {
      if (/[1-9.]/.test(button.name)) {
        setExpression(button.name.replace());
      } else {
        setExpression(result + button.name);
      }
    }
  }

  const calculate = () => {
    setResult(eval(expression));
    setExpression(prev => prev + "=")
  }

  const clearOne = () => {
    setExpression(prev => prev.substring(0, prev.length - 1));
    setResult(0);
  };

  const clearAll = () => {
    setExpression("");
    setResult(0);
  };

  let action;
  switch (button.id) {
    case 'equals':
      action = calculate;
      break;
    case 'back':
      action = clearOne;
      break;
    case 'clear':
      action = clearAll;
      break;
    default:
      action = display;
      break;
  }

  return (
    <button onClick={action} id={button.id} className={button.id} style={style}>
      <i style={style}>{button.name}</i>
    </button>
  )
}

ReactDOM.render(<App />, document.getElementById('root'));

Your browser information:

User Agent is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.164 Safari/537.36

Challenge: Build a JavaScript Calculator

Link to the challenge:

Actually it is tests 8,11,13 failing

What I don’t understand is that some tests require the input expression to display in the element with id=“display” and other tests require the output result to display in the element with id=“display”. I don’t see how I can combine these two into one element. And I have difficulty processing the expression to clean it up.

Here is the link to my codepen project:javascript calculator

Hello @danmikes ,

What I don’t understand is that some tests require the input expression to display in the element with id=“display” and other tests require the output result to display in the element with id=“display”. I don’t see how I can combine these two into one element.

I’m not sure if I understand this correctly, but maybe the concept of side effects[0] can be useful:

// One element with id="display"
const myDisplay = document.getElementById("display");

// function with side effects 
function fnInput() {
 let myInput = "1 + 1";
 myDisplay.textContent = myInput; 
}

// function with side effects 
function fnOutput() {
 let myOutput = "2";
 myDisplay.textContent = myOutput; 
}

As you can see, this is only one element that have (my)Input and (my)Output “combined”.


What is this code doing? why there is a value={expression} and a value={result}?

<div id="screen" className="screen">
        <input id="expression" className="expression" type="text" value={expression} placeholder="0" disabled></input>
        <input id="display" className="display" type="text" value={result} placeholder ="0" disabled></input>
      </div>

And I have difficulty processing the expression to clean it up.

You are using an event-action[1] approach:

  • Users supplies an event

  • Event-handlers will execute functions in response to an event

  • Elements of the user interface may respond to an event in different ways, depending on the context

  • if-else statements are used to determine the context and which functions are executed

  • Event-handlers must use information shared between user interface objects

  • To share information the following options are used:

    • Explicit global variables

    • A value contained in an user interface object

    • Other attribute of an object

I think that part of the problem is that there is no clarity on what are you testing (to get the context), an example:

...

  } else if (expression[expression.length - 1] == ".") {
      if (/[.]/.test(button.name)) {
        setExpression(prev => prev.substring(0, prev.length - 1));
      } else {
        setExpression(prev => prev.substring(0, prev.length - 1) + button.name);
      }
    }
....

why are you testing this in display? what exactly are you testing? because I can enter this: 1.1.1. without a problem (and I shouldn’t ).


Also, comments that are really useful to people that are reading your code (most of the time without a comment there is no way to know why the code exists).

Cheers and happy coding :slight_smile:

Notes:

[0] Side effect (computer science) - Wikipedia

In computer science, an operation, function or expression is said to have a side effect if it modifies some state variable value(s) outside its local environment, that is to say has an observable effect besides returning a value (the intended effect) to the invoker of the operation. State data updated “outside” of the operation may be maintained “inside” a stateful object or a wider stateful system within which the operation is performed.

[1] Event-driven programming - Wikipedia

In computer programming, event-driven programming is a programming paradigm in which the flow of the program is determined by events such as user actions (mouse clicks, key presses), sensor outputs, or message passing from other programs or threads. Event-driven programming is the dominant paradigm used in graphical user interfaces and other applications (e.g., JavaScript web applications) that are centered on performing certain actions in response to user input.

1 Like

Hello @erretres,

Thank you for taking so much time to look at my code. Your comments helped me a lot. And to realise I lack basic knowledge of javascript. I will rewrite the logic with your useful suggestions. The side effect is indeed a nice one!

I had two separate elements “expression” and “display”, so the one showed the expression (input) and the other the result (output). But I can remove the one with only result.

And I shall add some comments next time.

1 Like

You are welcome :slight_smile:

@erretres,

I have refactored the code. I think it is a little cleaner now and I added some comments.
The expression and result now show up on the display thanks to your suggestions.
The only thing remaining is to sanitise the expression in the display before it is evaluated. All logic for that should be in function const clean = () => …
I am now stuck at two things:
(1) How to write the function such that it cleans and updates the expression. Should I have it work directly on setExpression() or should I pass it a value and call setExpression() from const input = value => () => …
(2) How to replace the expression by the sanitised version. I am struggling with the regex / replace statement.
I also feel that the code const input = value => … and const output = value => … are not optimal. I tried to use async … await, but failed miserably.
I feel very stupid, because surely I should be able to find this somewhere, but I can’t.

the code pen link:

So now I thought to improve the logic by completely reformatting the code and splitting functions into many small ones. I am used to doing this in java. And I think it should work in javascript. But I ignore how to set and use state. I feel more and more ignorant.

I did it. I was overcomplicating things. Now I believe I have quite a simple and elegant solution to cleaning the expression.

(Make sure to format your code using back ticks via markdown)

Use the format below but add three backticks ontop and three on the bottom


It ends up looking like this:

const display = () => {
setExpression(prev => prev + button.name);
if (expression[expression.length - 1] == “=”) {
if (/[1-9.]/.test(button.name)) {
setExpression(button.name.replace());
} else {
setExpression(result + button.name);
}
}
}
1 Like

Thank you for the tip.

Hello @danmikes.
Sorry for the delayed response, I have not been able to connect.

First, congratulations I see that the app is passing all the tests :+1:

I did it. I was overcomplicating things. Now I believe I have quite a simple and elegant solution to cleaning the expression.

The code is much better now (and easier to read) :+1:


Sorry I should have been clearer when I wrote the example of side effects but (in this case) in Reactjs you should use expression/setExpression/React.useState and not try to access the DOM directly:

  // show expression in display
  const display = () => {
    document.getElementById("display").value = expression;
  }

Cheers and happy coding :slight_smile:

Thank you for coming back to me. You helped me a lot! Ciao. :wave: