2
\$\begingroup\$

I'm doing freeCodeCamp's Coding Interview Prep to improve my JavaScript skills. This challenge is called "Word Frequency", and is based on the Rosetta Code's entry of the same name.

The version I'm solving says:

Given a text string and an integer n, return the n most common words in the file (and the number of their occurrences) in decreasing frequency. Write a function to count the occurrences of each word and return the n most commons words along with the number of their occurrences in decreasing frequency.

  • The function should return a 2D array with each of the elements in the following form: [word, freq]. word should be the lowercase version of the word and freq the number denoting the count.
  • The function should return an empty array, if no string is provided.
  • The function should be case insensitive, for example, the strings "Hello" and "hello" should be treated the same.
  • You can treat words that have special characters such as underscores, dashes, apostrophes, commas, etc., as distinct words. For example, given the string "Hello hello goodbye", your function should return [['hello', 2], ['goodbye', 1]].

For this solution I'm trying to be concise and trying to use modern features of the language. Here is the code:

const wordSplit = (text) =>
  text
    .replace(/[.,:;?!]/g, '')
    .split(/\s/)
    .filter(word => word !== '')
    .map(word => word.toLowerCase())

const countWords = (text) => {
  const wordCount = {}

  const count = (word) => {
    wordCount[word] ? wordCount[word]++ : wordCount[word] = 1
  }
  wordSplit(text).forEach(word => count(word))

  return wordCount
}

const wordFrequency = (text = '', topn) =>
  Object
    .entries(countWords(text))
    .sort((a, b) => b[1] - a[1])
    .slice(0, topn)

An example of using the code:

console.log(wordFrequency("Don't you want to know what I don't know?", 3))

That will print:

[ [ "don't", 2 ], [ 'know', 2 ], [ 'you', 1 ] ]

I have some specific, optional, questions:

  • Do you consider this code easily readable and maintainable? If not, what would you change to improve this area?
  • Can this be written more concisely? (without sacrificing too much readability)
  • Can this code be improved using other features of the language? Perhaps more modern features?
  • Can we use the nullish coalescing operator (??) or the local nullish assignment operator (??=) to simplify the assignment and increment currently done in the ternary operator in the count function?

But I'm also very interested in anything else you can think of that can improve this implementation, or change it in interesting ways, including more efficient approaches, more readable solutions, best practices, code smells, patterns & anti-patterns, coding style, etc.

\$\endgroup\$
1
\$\begingroup\$

The code is easily readable and variables are well named. What you could do is have functions instead of lambdas, but that is just my personal preference. Also "wordSplit" is a function and would be a bit better named splitIntoWords(text) {...}

From the reuse and maintenance perspective the wordSplit fells like it is a little misplaced. It is not a major difference, but it makes countWords more reusable:

function wordFrequency(text = '', topn) {
  const words = splitIntoWords(text)
  const wordCount = countWords(words)
  return Object.entries(wordCount)
      .sort((a, b) => b[1] - a[1])
      .slice(0, topn)
}

You can also isolate the last line into a separate function and give it a name:

function pickNMostFrequent(wordCount, n) {
  return Object.entries(wordCount)
      .sort((a, b) => b[1] - a[1])
      .slice(0, topn)
}

function wordFrequency(text = '', topn) {
  const words = splitIntoWords(text)
  const wordCount = countWords(words)
  return pickNMostFrequent(wordCount, topn)
}

CountWords is the only place that feels awkward. What you can do to simplify it is the following (Yes, you can use nullish assignment):

function countWords(words) {
 return words.reduce((wordCount, word) => {
    wordCount[word] ??= 0
    wordCount[word]++
  }, {})
}
\$\endgroup\$
1
\$\begingroup\$

Tiny review;

  • functions ideally follow <verb><Subject> so wordSplit -> splitText
  • I think it is important to realize that when you do ff(v => f(v)) you can also do ff(f), or in this case you could do wordSplit(text).forEach(count) or even textSplit(text).forEach(updateWordCount) if we used better function names
  • ideally topn should be topN, and personally, I would just pass n
  • regex expressions without a comment are a misdemeanour
\$\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.