Generate every possible single word of three letters

Question

I was asked to generate every possible single word of three letters, so I wrote this script. How good is my code? Would it be better doing it using recursion or would that be over-engineering?

const getAllPossibleThreeLetterWords = () => {
      const chars = 'abcdefghijklmnopqrstuvwxyz'
      const arr = [];
      let text = '';
      for (let i = 0; i < chars.length; i++) {
            for (let x = 0; x < chars.length; x++) {
                  for (let j = 0; j < chars.length; j++) {
                        text += chars[i]
                        text += chars[x]
                        text += chars[j]
                        arr.push(text)
                        text = ''
                  }
            }
      }
      return arr
}
console.log(getAllPossibleThreeLetterWords());

Answer 1

It is very clear both what your code does and how it does it, this is a good thing. However, right now it is incredibly rigid. If requirements change more code will need to be changed than if you spent a bit more time on making it more generic (more on this later).

• You can avoid doing multiple assignments to text by combining the lines with text = chars[i] + chars[x] + chars[j]. This also removes the need to reset text to the empty string after adding the word.

• The variables i, x, and j seem to be randomly chosen. Not a big deal for loops this small, but it would be fairly easy to accidentally swap the order around. You might want to consider using a, b, and c instead.

---

What would you do if you were told to modify this method so that it generates 4 letter words? How about 20 letter words? Or just n letter words? While you could write a ton of for loops, it would be better to handle a more generic case. Whether you use recursion or not is entirely up to you.

To help you decide, here is a comparison.

Recursive, handling n letters.

let flatten = arr => arr.reduce((carry, item) => carry.concat(item), [])

let getAllWords = wordLength => {
    if (typeof wordLength != 'number') throw Error('wordLength must be a number')
    if (wordLength < 0) throw Error('wordLength must be greater than or equal to zero')
    if (wordLength == 0) return []

    let alphabet = 'abcdefghijklmnopqrstuvwxyz'.split('')
    let lengthen = word => alphabet.map(letter => word + letter)
    let addLetters = words => flatten(words.map(lengthen))
    let _getAllWords = (letters, words = alphabet, current = 1) => {
        return letters == current ? words : 
            _getAllWords(letters, addLetters(words), current + 1)
    }

    return _getAllWords(wordLength)
}

console.log(getAllWords(2))

Iterative, handling n letters.

let getAllWords = wordLength => {
    if (typeof wordLength != 'number') throw Error('wordLength must be a number.')
    if (wordLength < 0) throw Error('wordLength must be greater than or equal to zero.')

    let alphabet = 'abcdefghijklmnopqrstuvwxyz'.split('')
    let words = []
    if (wordLength != 0) words = alphabet

    for (let i = 1; i < wordLength; i++) {
        let temp = []
        words.forEach(word => {
            alphabet.forEach(letter => temp.push(word + letter))
        })
        words = temp
    }

    return words
}

console.log(getAllWords(2))

It really is somewhat of a matter of opinion which is more clean. I personally would likely use the recursive version with the helper functions pulled out of the function itself to improve readability.

Answer 2

Others have already improved the structure of the code so I'll talk about some smaller low-hanging-fruit improvements since it may not always be in your best interests to rewrite the code entirely.

Rather than chars, I would use alphabet since alphabet describes the purpose and contents of the string exactly.

Don't use abbreviations like arr, it's only two characters saved and an abbreviation will take the reader longer to parse than the full word they're used to. As it is, even array wouldn't be a good name. We can see that it returns an array, but it should be obvious what that array is intended to contain. Something like words would be an improvement and threeLetterWords would be better as it describes more clearly what the contents of the array is, and the intention that it is only supposed to contain three letter words.

text is also a poor name, word (or threeLetterWord) would be better but there are more problems with this than just the name. Firstly, it is only used in the innermost loop so it should be declared there. This is especially important in javascript with its often confusing scoping rules. It also removes the need to reset it at the end of the loop. Additionally, its usage could be improved by replacing the three assignments with a single one, i.e. let threeLetterWord = chars[i] + chars[x] + chars[j];. At this point I'd be tempted to remove threeLetterWord altogether and simply use threeLetterWords.push(chars[i] + chars[x] + chars[j]); but going to that extent can come down to personal preference.

Finally, the loop variable names are also poorly chosen. While i/j/k and x/y are common names for nested loop variables they shouldn't be mixed and where possible an actual name would improve the code, unless x/y are suitable names for the variables, for example when working with coordinates. Here I think (first|second|third)LetterIndex would be better.

Also, don't forget your semicolons. They might not be necessary but they're definitely useful.

In the end these improvements give the following code:

const getAllPossibleThreeLetterWords = () => {
    const alphabet = 'abcdefghijklmnopqrstuvwxyz';
    const threeLetterWords = [];

    for (let firstLetterIndex = 0; firstLetterIndex < alphabet.length; firstLetterIndex++) {
        for (let secondLetterIndex = 0; secondLetterIndex < alphabet.length; secondLetterIndex++) {
            for (let thirdLetterIndex = 0; thirdLetterIndex < alphabet.length; thirdLetterIndex++) {
                arr.push(
                    alphabet[firstLetterIndex]
                    + alphabet[secondLetterIndex]
                    + alphabet[thirdLetterIndex]);
            }
        }
    }

    return threeLetterWords;
}
console.log(getAllPossibleThreeLetterWords());

This code still isn't as good as the other answers that use a recursive method, but it demonstrates some easy changes to make that improve the comprehensibility of the code without changing its overall structure. The only potentially risky risky change is the removal of text but in this case references to it were easy to identify and change safely. Now that the code is easier to understand it would be easier to make more significant changes to its structure with more confidence.

Answer 3

To iterate on the answer Gerrit0 provided, I found combining his two snippets results in most readable code:

const alphabet = 'abcdefghijklmnopqrstuvwxyz'.split('');

const makeWords = wordLength => {
    if (wordLength < 1) {
        return [];
    }

    let counter = 1;
    let words = [...alphabet];

    const addLetter = word => alphabet.map(letter => word + letter);
    const flatten = (sum, curr) => sum.concat(curr);

    while (counter !== wordLength) {
        words = words.map(addLetter).reduce(flatten, []);

        counter++;
    }

    return words;
};

console.log(makeWords(3));