patternjavascriptMinor
Check if string is or can be made into a "valid string"
Viewed 0 times
canmadeintovalidcheckstring
Problem
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:
I'd appreciate any comments and suggestions as to how my code can be improved.
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 1
|| diffOnes[0] > (compareVal + 1)){
return false;
}
return true;
}Solution
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:
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
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 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.
Code Snippets
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';
}console.log(isValidString(testString));Context
StackExchange Code Review Q#148611, answer score: 4
Revisions (0)
No revisions yet.