There was a bug with my calc when you tried .1+.2+.3
or similar.
The 2nd number was not detecting the decimal point.
Should be fixed now.
I like how you used different colours to make it look like a real-world calculator that might be sitting on my desk.
There were a couple of little things I noticed.
- I can’t multiply/divide by a negative number unless it’s the first number entered.
5*-5
is calculated as5-5
. - There is a bug that gives odd results after a few computations because the entire expression is being evaluated instead of the prior result against the new expression. for example
10 - 8
evaluates to2
if I continue with* 100
I’d expect200
but I get-790
. - After an expression has been evaluated I’d expect leading with a number instead of an operator to clear the expression and start fresh. But its merging it with the last number of the current expression.
5 * 2
evaluates to10
and then I enter5 / 2
and I get the result of5*25/2
. - Your calculator doesn’t seem to like numbers following
0
,5 * 201
is entered as5 *21
.
I feel like every time I open my calculator up I find a new bug with it, I actually just rebuilt it with React and found some fresh bugs so now my handleClick function is a huge mess of if/elses. Which is probably why I was able to find a couple of bugs in yours, I’m still in full destroy mode haha.
I really like the use of a switch statement here I wish I would have thought of that it makes perfect sense to keep the code more manageable.
Thanks for the thorough testing
This is one of those challenges that seems endless in the ability to create bugs. It could do will a test library for users to go through because thinking of all the side conditions is difficult.
Regarding a couple: 5*-5 could just as easily be entered as -5*5 which works. So, yes it’s a bug but you can still get the correct answer
The way I was taught Maths 10 - 8 * 100 does = 790 because of BODMAS rules. Multiplication and division takes precedence over addition & subtraction but I see the problem. The calculation is left in the input window so adding more numbers spoils the result.
After doing this and helping someone else, I thought of a better way to build up the expression so I will probably do a React version like you as a version 2. Basically I think using an array to build up the expression would have been better.
Thanks again for the feedback
Sure lol but this kind of goes along with the other one, if I want to multiply by -5 after getting the result of a previous expression I’m kind of at a loss.
I didn’t know they used a different acronym overseas for order of operation, I was taught PEMDAS. huh TIL.
Currently I’m using a string to store and display the expression and parsing it as an array before passing it to my final calculation, if you want to take a look here’s a link. But my handleClick which is where I’ve tinkered with finding all the edge cases has become quite ugly and kind of hard to follow
handleClick(e) {
const char = e.target.innerText;
const numArr = this.state.calculation.split(/[()%÷x\-+]/);
const ops = '%÷x-+';
let temp = '';
if (char === '.') {
if (numArr[numArr.length -1].includes('.')) {
return;
} else if (this.state.isEvaluated) {
this.clearDisplay();
temp = char;
} else {
temp = char;
}
} else if (ops.indexOf(char) > -1) {
if ((this.state.calculation.length < 1) && (char !== '-')) {
temp = '';
} else if ((char === '%') && (this.state.calculation[this.state.calculation.length -1] === '%')) {
temp = '';
} else if (this.state.calculation[this.state.calculation.length - 1].match(/[0-9)%]/) != null) {
temp = char;
} else {
if (char === '-') {
if (ops.includes(this.state.calculation[this.state.calculation.length -2])) {
temp = '';
} else {
temp = char;
}
} else {
if (ops.indexOf(this.state.calculation[this.state.calculation.length-2]) > -1) {
return this.setState({
calculation: this.state.calculation.slice(0,-2) + char,
output: this.state.output.slice(0,-2) + char,
})
}
return this.setState({
calculation: this.state.calculation.slice(0,-1) + char,
output: this.state.output.slice(0,-1) + char,
});
}
}
} else if ((char === '(') && (this.state.calculation === '')) {
temp = char;
}
else if (((char === '(') && (this.state.calculation[this.state.calculation.length -1].match(/[0-9]/) != null )) ||
((char.match(/[0-9]/) != null) && (this.state.calculation[this.state.calculation.length -1] === '%'))) {
temp = `x${char}`;
} else if ((char.match(/[0-9]/) != null) && (this.state.isEvaluated)) {
return this.setState({
calculation: char,
output: char,
isEvaluated: false,
});
} else {
temp = char;
}
this.setState({
calculation: this.state.calculation + temp,
output: this.state.output === '0' ? temp : this.state.output + temp,
isEvaluated: false,
});
}
I’ve just kind of plugged edge cases in where I could fit them instead of really rethinking the function as a whole. I think a switch statement would make it much more readable. Comments probably wouldn’t hurt either lol but that’s another bag of worms entirely.
Looking good so far! My advice, try not to spend too long on it because it can become very time consuming!
Ok I think I have fixed the 6*-3
type equation now.
I also added typing as an option although I haven’t done keys for AC and CE yet.
Feel free to play with it:
Now I can chain multiple operators together by pressing 6*-*-/-*3
I get 6**/*3
Also I cant continue an equation by leading with an operator after the previous expression is evaluated.
Yep, it’s a coder’s nightmare! Needs TDD.
Anyway, I think I squashed that one
6*-*-/-3 I get 6*/*3
Not sure about the 2nd problem. How is the typing?