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
in finalString
. i
and j
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); });