My code will generate a spelling mistake inside of a string 50% of the time.
It will retrieve a letter from a random index in the string like, "t" and then duplicate that letter like, "tt" and store it in the spelling mistake variable.
It will then replace, "t" with "tt" in the string to replicate a spelling mistake. (50% of the time.)
How can I improve my code to make it complete the same task, but with less lines of code and using the least amount of resources possible?
(function() {
function replaceStr(str, pos, value) {
var arr = str.split('');
arr[pos] = value;
return arr.join('');
}
var myString = "Stack Overflow";
var letterIndex = Math.floor(Math.random() * myString.length); // Example: 1
var letter = myString.charAt(letterIndex); // Example: "t"
var mistake = letter + letter; // Example: "tt"
// 0 -> 9 (coin toss)
if (Math.floor(Math.random() * 10) >= 5) {
return replaceStr(myString, letterIndex, mistake);
} else {
return myString;
}
})();
Result from 10 runs:
Stack Overflow
Stackk Overflow
Stack Overflow
Sttack Overflow
Stack Overflow
Stack Oveerflow
Stack Overrflow
Stack Overfllow
Stack Overflow
Stack Oveerflow
-
2\$\begingroup\$ Does it matter that duplicating a letter doesn't always generate a misspelled word? E.g., duplicating t in later produces latter which is another correctly spelled word. \$\endgroup\$Joshua Taylor– Joshua Taylor2016年03月02日 22:18:01 +00:00Commented Mar 2, 2016 at 22:18
-
\$\begingroup\$ I've never considered the potential applications for a typo machine. Interesting. \$\endgroup\$JamesFaix– JamesFaix2016年03月02日 23:53:09 +00:00Commented Mar 2, 2016 at 23:53
-
\$\begingroup\$ Sometimes less lines of code and less resources are opposite things. \$\endgroup\$Pharap– Pharap2016年03月03日 05:18:15 +00:00Commented Mar 3, 2016 at 5:18
-
3\$\begingroup\$ But, why would you need to make a spell error? Just curious :-) \$\endgroup\$user15331– user153312016年03月03日 06:57:39 +00:00Commented Mar 3, 2016 at 6:57
-
2\$\begingroup\$ @xyz reassons... \$\endgroup\$User1000547– User10005472016年03月03日 14:50:18 +00:00Commented Mar 3, 2016 at 14:50
4 Answers 4
Break it down into one function that randomly repeats a character, and another function which randomly executes that function 50% of the time:
function repeatRandomChar(str) {
var i = Math.floor(Math.random() * str.length);
return str.slice(0, i+1) + str.slice(i);
}
function randomTypo(str) {
return Math.random() > 0.5 ? repeatRandomChar(str) : str;
}
Now you can just call randomTypo('Stack Overflow')
10 times to reproduce your sample data.
Note also that the original code has needless multiplication, in that
Math.random() > 0.5
is equivalent to:
(Math.random() * 10) >= 5
-
\$\begingroup\$ Your elimination of the needless multiplication with
Math.random() < 0.5
is also worth pointing out. \$\endgroup\$200_success– 200_success2016年03月02日 19:39:11 +00:00Commented Mar 2, 2016 at 19:39 -
2\$\begingroup\$ Wouldn't it be more efficient to avoid an extra concatenation by replacing
str.slice(0, i+1) + str.charAt(i) + str.slice(i+1)
withstr.slice(0, i+1) + str.slice(i)
? \$\endgroup\$Michael Urman– Michael Urman2016年03月02日 22:47:33 +00:00Commented Mar 2, 2016 at 22:47 -
\$\begingroup\$ @MichaelUrman Love it. I made the change. \$\endgroup\$Jonah– Jonah2016年03月02日 23:04:44 +00:00Commented Mar 2, 2016 at 23:04
-
5\$\begingroup\$ Actually
Math.random() >= 0.5
would be equivalent to(Math.random() * 10) >= 5
. In this case it doesn't matter whether you check >= or <, but for any other number (say 7 and 0.7) it does. \$\endgroup\$Stack Exchange Broke The Law– Stack Exchange Broke The Law2016年03月03日 02:07:27 +00:00Commented Mar 3, 2016 at 2:07 -
\$\begingroup\$ One may also want to check for whites. \$\endgroup\$DarioP– DarioP2016年03月03日 14:50:20 +00:00Commented Mar 3, 2016 at 14:50
This code is at the end of your function:
// 0 -> 9 (coin toss) if (Math.floor(Math.random() * 10) >= 5) { return replaceStr(myString, letterIndex, mistake); } else { return myString; }
Half of the time on average, all the code above this code will not matter, because you simply return myString
unchanged. So it would be better to move this condition higher, before the other logic, such as picking a random position to introduce the mistake, etc.
And instead of splitting the string and changing a position, perhaps it's more efficient to work with substrings.
Something like this:
function addMistake(myString) {
// 0 -> 9 (coin toss)
if (Math.floor(Math.random() * 10) < 5) {
return myString;
}
function replaceStr(str, pos, value) {
return str.substr(0, pos) + value + str.substr(pos + 1);
}
var letterIndex = Math.floor(Math.random() * myString.length); // Example: 1
var letter = myString.charAt(letterIndex); // Example: "t"
var mistake = letter + letter; // Example: "tt"
return replaceStr(myString, letterIndex, mistake);
}
You can reduce your code, merely avoiding useless complications, like this:
(also improved using @Michael Urman's idea to compact the substitution statement)
function doMistake(str) {
if (Math.floor(Math.random() * 10) >= 5) {
var letterIndex = Math.floor(Math.random() * str.length);
str = str.substr(0, letterIndex + 1) + str.substr(letterIndex);
}
return str;
}
Here it is working with the example provided by the OP:
function doMistake(str) {
if (Math.floor(Math.random() * 10) >= 5) {
var letterIndex = Math.floor(Math.random() * str.length);
str = str.substr(0, letterIndex + 1) + str.substr(letterIndex);
}
return str;
}
for (var i = 0; i < 10; i++) {
console.log(doMistake('Stack Overflow'));
}
-
3\$\begingroup\$ Wouldn't it be more efficient to avoid an extra concatenation by replacing
str.substr(0, letterIndex) + str.charAt(letterIndex) + str.substr(letterIndex)
withstr.substr(0, letterIndex + 1) + str.substr(letterIndex)
? \$\endgroup\$Michael Urman– Michael Urman2016年03月02日 22:48:22 +00:00Commented Mar 2, 2016 at 22:48 -
\$\begingroup\$ @MichaelUrman Wow, nice improvement, and a clever one! I edit my answer with it. Thanks! \$\endgroup\$cFreed– cFreed2016年03月02日 22:58:00 +00:00Commented Mar 2, 2016 at 22:58
One could String.replace() with an replacement-function. Moreover I have incorporated a few of the suggestions already made.
var myString = "Stack Overflow";
var letterIndex; // Example: 1
var letter; // Example: "t"
function replacer(match, offset, string) {
if (offset === letterIndex) {
return match + match;
}
}
// 0 -> 9 (coin toss)
if (Math.random() >= 0.5) {
letterIndex = Math.floor(Math.random() * myString.length);
letter = myString.charAt(letterIndex);
return myString.replace(letter, replacer);
} else {
return myString;
}