# Check if string is or can be made into a "valid string"

The Challenge:

A valid string is one in which all distinct characters occur the same number of times. Given a string S, check if the string can be made valid by removing less than or equal to one character. If yes, output "YES". Otherwise output "NO".

Constraints:

• $1 \le \lvert S \rvert \le 10^5$
• String S contains lowercase letters only (a-z)

I'd appreciate any comments and suggestions as to how my code can be improved.

function processString(str) {
if(str.length === 1){
console.log("YES");
}
else{
const letterCount = buildCount(str.trim());
console.log(checkValid(letterCount) ? "YES" : "NO");
}
}

function buildCount(str){
const letterCount = {};
const len = str.length;
for (let i = 0; i < len; i++) {
letterCount[str[i]] = (letterCount[str[i]] || 0) + 1;
}
return letterCount;
}

function checkValid(letterCount){
const totals = [];
for(let key of Object.keys(letterCount)){
totals.push(letterCount[key]);
}
//only one can be different- and it has to be more than the others
//if always compare to the first,
//  doesn't work if that is the one that is more
const compareVal = Math.min(totals[0], totals[1]);
const diffOnes = totals.filter(function(val){
return val !== compareVal;
});
if(diffOnes.length > 1
|| diffOnes[0] > (compareVal + 1)){
return false;
}
return true;
}


Your logic is fine. I think that a function of this sort does not necessarily need to be broken down into different functions. This function would still serve a single responsibility and without any wider need of such really specific functions in a wider application context, I see no reason to break it apart like this.

I think what you have can be simplified to something like this:

function isValidString(str) {
var strArray = str.split('');
var counter = strArray.reduce(
(cnt, letter) => { cnt[letter] = (cnt[letter] || 0) + 1; return cnt; }
,{}
);
var i = 0;
var compareVal;
var diffFound = false;
for (letter in counter) {
i++;
if (i === 1) {
compareVal = counter[letter];
continue;
}
if (counter[letter] === compareVal)
continue;
if (diffFound)
return 'NO';
diffFound = true;
if (Math.abs(counter[letter] - compareVal) > 1)
return 'NO';
if (counter[letter] < compareVal) {
if (i === 2) {
compareVal = counter[letter];
} else {
return 'NO';
}
}
}
return 'YES';
}


Note here we use reduce function to build the letter counter object, eliminating need for separate function. We also slightly optimize the logic to iterate and evaluate the counter, allowing for early exit from that iteration as soon as an invalid condition is encountered.

I probably would not include console logging within the function itself, as I think the function should do only one thing, which is determine if string is valid, not output it as well. Obviously, it is trivial to log the output of this function like

console.log(isValidString(testString));


Be careful in ensuring your function and variables names are meaningful. Your functions are vaguely named and do not specifically convey what caller should expect to happen. 'letterCount' as a variable name would lead me to expect an integer value to be contained therein, not an object with letter counts in it. A simple change to 'letterCounter' would make tremendous difference in conveying meaning of this variable.

• I would argue that as a rule of thumb that having smaller functions keeps it more readable and a bit more maintainable by limiting the scope of all the variables. Commented Dec 2, 2016 at 5:24