When writing maintainable code, I'd recommend to declare variables in narrow scopes, if possible. If you do something like:
var inputStr, inputsArray, numsArray, arrayStrNumber, isAdd, isSub, isMul, isDiv;
at the very beginning, then on every line throughout the rest of the program, you'll have to keep in mind the possible values for all of those variables. If the script is long, this can be a problem. Consider encapsulating related functionality into separate functions instead - for example, you could turn the input into an array in one function, sort it in another, and then finally iterate through it to produce the result in a third. That way, for example, inputStr will only be visible to the first function (as an argument), and neither of the first two functions will have isAdd etc visible to them. Separating functionality into smaller self-contained sections makes code a lot more readable.
Since it's 2020, it would be nice to write source code in modern syntax - at least in ES2015. Modern syntax generally makes code more concise, easier to read, and less buggy. (I'll be using modern syntax in suggestions below.)
Consider always using strict mode - you have some variables that you never declare (strArray and x), which means that they get implicitly assigned to the global object. This is an easy source of bugs, not to mention inelegant; strict mode will throw an error when this sort of thing happens, allowing you to fix it on sight.
When you have to iterate over an array or a string, it's nice to be able to immediately work with each item being iterated over immediately, rather than having to mess with indicies. Both strings and arrays have iterators (which iterate over either each character, or each array item), so you can do this concisely with for..of. For example:
for (let i = 0; i < inputsArray.length; i++) {
const inputValue = inputsArray[i];
can be replaced with
for (const inputValue of inputsArray) {
You do
isNaN(numsArray[i]) ?
(numsArray[i] = "" + inputValue[x] + "") :
(numsArray[i] += "" + inputValue[x] + "");
The conditional operator should only be used when you need an expression which is identified conditionally, like console.log(cond ? '1' : '2'). If the expression that results from using the conditional operator isn't being used, like in your code, it would be more readable to remove it and use if/else instead. (It's fine for minifiers to do that, but minified code isn't intended to be read - source code that developers write should be as readable as possible). There's even a linting rule to help you automatically identify and fix this sort of thing.
So, you have a string which is composed of numbers, plus one letter somewhere inside it, and you need to separate them. If you want to do this concisely, you might consider using replace to replace the letter character with the empty string, and use a callback to assign the matched value (the letter) to an outer variable, like this:
function separateWord(word) {
let char;
const num = Number(
word.replace(
/[a-z]/i,
match => {
char = match;
return ''; // replace this match with the empty string
}
)
);
return { char, num };
}
console.log(separateWord('123b456'));
On a broader scale, using the above function, I think the first section of the code which turns each word into a sorted array would be easier to manage if you used an object indexed by character instead, whose values are arrays of numbers. Eg, for the above 123b456, that would result in the following object:
{
b: [123456]
}
Iterate over each word, pushing to an array on the object, creating it first if necessary. Then, the object properties can be arranged lexiographically by sorting its entries (Object.entries returns an array of entries, where an entry is an array containing the key and the value, so just compare the keys in the .sort). This makes things so much shorter and cleaner:
function getOrderedNumbers(numbersByChar) {
return Object.entries(numbersByChar)
.sort((a, b) => (a[0] > b[0]) - 0.5) // Order entries alphabetically
.map(entry => entry[1]) // Take only the value of each entry
.flat(); // Turn the array of arrays of numbers into a single array of numbers
}
function parseString(inputStr) {
const numbersByChar = {};
for (const word of inputStr.split(' ')) {
const { char, num } = separateWord(word);
if (!numbersByChar[char]) {
numbersByChar[char] = [];
}
numbersByChar[char].push(num);
}
const numbersArr = getOrderedNumbers(numbersByChar);
return doMath(numbersArr);
}
For the doMath function, keeping track of 4 separate booleans and performing 4 different if checks is a bit verbose for the task at hand. You might consider using an array of functions instead, and using the modulo operator to identify which function to call:
function doMath(numbersArr) {
const fns = [
(subtotal, num) => subtotal + num,
(subtotal, num) => subtotal - num,
(subtotal, num) => subtotal * num,
(subtotal, num) => subtotal / num,
];
// Remove first value so operation can start with + with first and second value:
let subtotal = numbersArr.shift();
numbersArr.forEach((num, i) => {
subtotal = fns[i % 4](subtotal, num);
});
return subtotal;
}
console.log(doMath([1, 2, 4, 5])); // ((1 + 2) - 4) * 5
reduce would be more appropriate than forEach to transform the array of numbers into a single number, but if you're a beginner, you probably prefer the forEach version since it's more intuitive:
function doMath(numbersArr) {
const fns = [
(subtotal, num) => subtotal + num,
(subtotal, num) => subtotal - num,
(subtotal, num) => subtotal * num,
(subtotal, num) => subtotal / num,
];
// Remove first value so operation can start with + with first and second value:
const initialValue = numbersArr.shift();
return numbersArr.reduce((subtotal, num, i) => fns[i % 4](subtotal, num), initialValue);
}
console.log(doMath([1, 2, 4, 5])); // ((1 + 2) - 4) * 5
Put it all together:
'use strict';
console.log(doMath("10a 90x 14b 78u 45a 7b 34y"));
function separateWord(word) {
let char;
const num = Number(
word.replace(
/[a-z]/i,
match => {
char = match;
return ''; // replace this match with the empty string
}
)
);
return { char, num };
}
function getOrderedNumbers(numbersByChar) {
return Object.entries(numbersByChar)
.sort((a, b) => (a[0] > b[0]) - 0.5) // Order entries alphabetically
.map(entry => entry[1]) // Take only the value of each entry
.flat(); // Turn the array of arrays of numbers into a single array of numbers
}
// Entry point:
function doMath(inputStr) {
const numbersByChar = {};
for (const word of inputStr.split(' ')) {
const { char, num } = separateWord(word);
if (!numbersByChar[char]) {
numbersByChar[char] = [];
}
numbersByChar[char].push(num);
}
const numbersArr = getOrderedNumbers(numbersByChar);
return getTotal(numbersArr);
}
function getTotal(numbersArr) {
const fns = [
(subtotal, num) => subtotal + num,
(subtotal, num) => subtotal - num,
(subtotal, num) => subtotal * num,
(subtotal, num) => subtotal / num,
];
// Remove first value so operation can start with + with first and second value:
let subtotal = numbersArr.shift();
numbersArr.forEach((num, i) => {
subtotal = fns[i % 4](subtotal, num);
});
return Math.round(subtotal);
}
Unfortunately, the latest JS version that Codewars supports is Node 8, which is old, end-of-life, and probably shouldn't be used - it doesn't support Array.prototype.flat. An alternative to achieve the same functionality is to spread into concat instead:
arr.flat()
can be replaced by
[].concat(...arr);
