Today, I tried to write a simple function that would display the characters of my string one by one by iterating over a string containing the letters of the alphabet and showing the steps on the screen.
Just out of personal curiosity and fun: how would you write it? Is there something you would optimize, both in terms of readability and complexity?
I'll provide both the sandbox of the desired behavior and the code for the function.
const buildString = (finalString) => {
let i = 0;
let j = 0;
const tempStringArray = [];
const cycleLength = finalString.length;
const interval = setInterval(() => {
const temp = finalString[i];
const char = alphabet[j];
tempStringArray.push(undefined);
tempStringArray[i] = char;
name.value = tempStringArray.join('');
if (temp === char) {
j = 0;
i++;
} else {
j++;
}
if (i >= cycleLength) clearInterval(interval);
}, 50);
}
1 Answer 1
Most of the variable names are badly chosen.
buildString
doesn't really express what it does.name
doesn't match its use (unless you happen to output a name, but it's hard-coded into the function and you'll hardly be outputing a name each time. See also below.)- I don't see the meaning of
final
infinalString
. i
andj
don't have a connection to the variables they index.temp
is always a bad name/prefix, because all variables are temporary.- etc.
The arrow function syntax should only be used when in-lining a function. For "top level" functions the function
keyword clearly shows what is being defined.
tempStringArray.push(undefined)
is pointless. In JavaScript array items don't need to exist in order to write to them. Also you are creating far too many items. If, then you'd only need it when i
is incremented.
tempStringArray
itself isn't really needed. It would be easier just to output the first i - 1
characters of the original string plus alphabet[j]
.
Hard-coding name
into the function is a bad idea.
- For one, as general rule in programming, functions should never access outside variables (
alphabet
is okay, because it's an actual constant). - The function is unnecessarily bound to Vue.
- You only can use the function only once in the same component.
Here's my solution:
const alphabet = " abcdefghijklmnopqrstuwyz";
const waterfallOutput = ref("");
function waterfall(string, output, speed = 50) {
let stringIndex = 0;
let alphabetIndex = 0;
const interval = setInterval(() => {
const stringChar = string[stringIndex];
const alphabetChar = alphabet[alphabetIndex];
// TODO Add escape clause if `alphabetIndex` is too large, because the character is missing in `alphabet`
output(string.substr(0, stringIndex - 1) + alphabetChar);
if (stringChar === alphabetChar) {
alphabetIndex = 0;
stringIndex++;
} else {
alphabetIndex++;
}
if (stringIndex >= string.length) clearInterval(interval);
}, speed);
};
onMounted(() => {
waterfall("hello world", s => waterfallOutput.value = s);
});
-
\$\begingroup\$ I was checking it our, and I found a small error: the output excludes the penultimate character of the input string. By correcting the substr function and removing the -1, I resolved it. \$\endgroup\$Luca Natale– Luca Natale2023年11月07日 15:05:49 +00:00Commented Nov 7, 2023 at 15:05