trjhtr

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 😙!

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)