RegExp to the rescue.
First there is the problem for the review. The function is inconsistent and that makes it hard to know what it really must do.
Consider the test below if given the following inputs
"3 + 7" >> 10
"3 - 7" >> -4
"3- 7" >> undefined should be -4
"3 -7" >> undefined should be -4
"3-7" >> undefined should be -4
"3 - 7.9" >> -4 should be -5 as 7.9 is rounded to 8
"3 - - 7" >> -4 should be 10 as 3 - (-7) is 3 + 7
"3 - - 7" >> -4 should be 10 as 3 - (-7) is 3 + 7
"3 a 7" >> undefined correct
"3 - a 7" >> -4 should be undefined
??? not at all what one would expect
The review part
You got the first two lines...
function Calculator() {
this.calculate = function(sum) {
....and the last two...
}; // the ; not really needed after }
};
...correct. But all the rest is not helping solve the problem.
Step by step.
So lets step through the code
Set result would be undefined
this.summ = 0;
Split and vet numbers. Could have used isNaN()
and use local variables rather than object properties var sumArr
and var newSum
this.sumArr = sum.split(' ');
this.newSum = this.sumArr.filter(function(number) {
return number > 0;
});
Unneeded loop????
for (var i = 0; i < this.sumArr.length; i++) {
Check operator use ===
rather than ==
if (this.sumArr[i] == '+') {
Parse number and return result. parseInt
need a radix 10
return this.summ = parseInt(this.newSum[0]) + parseInt(this.newSum[1]);
} else if (this.sumArr[i] == '-') {
Parse number and return result
return this.summ = parseInt(this.newSum[0]) - parseInt(this.newSum[1]);
}
}
A better way
Vet
The function must first vet the string and return undefined if it is not a valid calculation.
You can do that with a RegExp.
The simple regExp /[^0-9\-+ .]/
- the
[
...]
mean any of,
- the
^
converts the [
...]
to means not any of,
- the
0-9
means digits "0", "1", "2", "3", "4", "5", "6", "7", "8", "9"
the \-
for character "-",(because -
has special meaning you must add the forward slash)
the + .
means "+", " ", and "."
So the expression will test if not any of "0123456789-+ ." are in the string, and can be used to vet the input and return undefined
if(/[^0-9\-+ .]/.test(sum)) { return undefined }
// not undefined is the default return so the line above is the same as
if(/[^0-9\-+ .]/.test(sum)) { return }
We could create an even more complex RegExp that would vet things like "1+1+2" but baby steps first.
Now that we have removed most of the invalide expressions on to parsing the string.
Remove irrelevant content
First remove any irrelevant content. That would be any spaces as they are not needed. Again a RegExp comes in handy. / /g
this means find /
" " /
space and the g
means global (all)
sum = sum.replace(/ /g,""); // remove all spaces
Find the operator
Now we can cut the sum in half to find the two numbers. We know that after the first number we need a "-" or "+".
Again a RegExp is used to find the location and value of the first "+" or "-" after the first number. /[0-9\.][+-]/
find and "0-9" or "." followed by "+" or "-"
const result = /[0-9.][+\-]/.exec(sum);
// if result is null then we have not found a digit followed by + -
// This means the string is invalid
if(result === null){ return } // return undefined
// the index of the found location is of the number before the + -
var num1 = sum.substr(0,result.index+1); // get number before
var operator = sum[result.index + 1]; // get the operator
var num2 = sum.substr(result.index + 2); // get the number after.
Now we have almost everything. There is still a chance of the two numbers being misinformed (eg "0.0.0" or second number as "0+10") but we can use the function isNaN
(is Not a Number) to vet these
Vet again
if(isNaN(num1) || isNaN(num2)) { return } // return undefined is one or both not numbers
Do the calculation
Now convert to the numbers. You should never use parseInt unless you are converting from a different radix (base). Use Number to parse a number and the use Math.floor
, Math.round
, Math.ceil
, or Math.trunc
to do the conversion to integer you want.
num1 = Math.round(Number(num1));
num2 = Math.round(Number(num2));
And now the result
if(operator === "-") { return num1 - num2 }
return num1 + num2;
And that is the logic.
Create the object Calculator
We can put it into an object with the method calculate
and then run a good set of tests. You should always test for errors not just for good results.
function Calculator() {
this.summ = undefined;
this.calculate = function(str) {
this.summ = undefined;
// vet out bad characters
if (/[^0-9\-+ .]/.test(str)) { return }
// remove spaces
str = str.replace(/ /g,"");
// find operator
const result = /[0-9.][+\-]/.exec(str);
if (result === null) { return }
// get numbers and operator
var num1 = str.substr(0, result.index + 1);
const operator = str[result.index + 1];
var num2 = str.substr(result.index + 2);
// check numbers
if (isNaN(num1) || isNaN(num2)) { return }
// parse and round numbers
num1 = Math.round(Number(num1));
num2 = Math.round(Number(num2));
// return result
if (operator === "-") { return this.summ = num1 - num2 }
return this.summ = num1 + num2;
}
}
// and test the results
var calc = new Calculator;
function log(val){
console.log("Result of " + val + " = " + calc.calculate(val));
}
log("3 + 7");
log("3 - 7");
log("3- 7");
log("3 -7");
log("3-7");
log("-3-7");
log("3 - 7.9");
log("3 - - 7");
log("3 - a 7");
log("3 a 7");
log("3-7-");
log("3-7-1");
Links
- Regex 101 is a helpful site to help get your head around regular repressions
- For all things javascript JavaScript MDN is a great resource.