4
\$\begingroup\$

I'm trying to create a program that will try to build a target string character by character by generating random characters. I have come up with the following Nodejs program with a little bit of Google help:

const args = process.argv.slice(2)

const targetWord = args[0]
let builtWord = ''

function getRandomChar() {
    return String.fromCharCode(Math.floor(Math.random() * (126 - 32 + 1)) + 32)
}

function updateWord() {
    const {length} = builtWord
    const nextChar = getRandomChar()
    process.stdout.write('\r')
    process.stdout.write(builtWord)
    process.stdout.write(nextChar)
    if (targetWord[length] === nextChar) {
        builtWord += nextChar
    }
}

const sleep = ms => new Promise(resolve => setTimeout(resolve, ms))

async function main() {
    while (true) {
        updateWord()
        if (builtWord === targetWord) {
            break
        }
        await sleep(1000 / 16)
    }
}

main()

What are your thoughts on it, anything I can improve or do differently? Thanks 😙!

\$\endgroup\$
3
\$\begingroup\$

Extract magic numbers into constants

  • For instance, what do 126 and 32 represent in the following line?

    return String.fromCharCode(Math.floor(Math.random() * (126 - 32 + 1)) + 32)
    

    It is much better to extract magic numbers into constants, i.e. to annotate them with a semantically meaningful name:

    const HIGHEST_ASCII_PRINTABLE_CHARACTER = 126;
    const LOWEST_ASCII_PRINTABLE_CHARACTER = 32;
    

    But even when you replace the numbers by their constant's name, it isn't immediately clear what you're doing with -32, +32, Math.random() and Math.floor() there. Even if you are accustomed to random number generation, a dedicated function call is more readable with less redundancy and in fact is easier to reason about:

    return String.fromCharCode(getRandomInt(LOWEST_ASCII_PRINTABLE_CHARACTER, HIGHEST_ASCII_PRINTABLE_CHARACTER));
    

    Especially, with your old code, I had to look up Math.random() on MDN to make really sure that is produces a uniform distribution. Now you can just add a docblock comment to getRandomInt indicating that property.

  • await sleep(1000 / 16) also features magic numbers.

Miscellaneous remarks

  • Using async-await for waiting after each loop iteration is a good use case and really makes the code cleaner!

  • Extracting only a single property with the object destructuring syntax (const {length} = builtWord) seems a bit unidiomatic to me as opposed to const length = builtWord.length. Apart from that, you are using length only once anyway, so I would drop that variable all together.

  • Try to avoid global variables and interdependencies.

    • For example, builtWord as well as targetWord are shared (and the former even modified!) by main and updateWord. I would suggest making updateWord accepting both as arguments and returning the possibly newly built word:

      function tryUpdateWord(currentBuiltWord, targetWord) {
          // This is a good moment to introduce a sanity check as well
          if (currentBuildWord.length >= targetWord.length) {
            throw new Error('<todo>');
          }
      
          const nextChar = getRandomChar()
          process.stdout.write('\r')
          process.stdout.write(currentBuiltWord)
          process.stdout.write(nextChar)
          if (targetWord[currentBuildWord.length] === nextChar) {
              return currentBuiltWord + nextChar
          }
          else {
              return currentBuiltWord;
          }
      }
      
    • Since the interdependencies have now been eliminiated, I would also move the part reading the process arguments into main():

      async function main() {
          const targetWord = process.args[2]
          let builtWord = ''
      
          const SLEEP_PER_ITERATION = 1000 / 16;
          while (true) {
              builtWord = tryUpdateWord(builtWord, targetWord)
              if (builtWord === targetWord) {
                  break
              }
              await sleep(SLEEP_PER_ITERATION)
          }
      }
      
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.