2
\$\begingroup\$

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.

Sandbox

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); } 
\$\endgroup\$

    1 Answer 1

    3
    \$\begingroup\$

    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); }); 
    \$\endgroup\$
    2
    • \$\begingroup\$i do appreciate this so much, thank you!\$\endgroup\$CommentedNov 7, 2023 at 14:26
    • \$\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\$CommentedNov 7, 2023 at 15:05

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.