Find the 'n' most frequent words in a text, aka word frequency

Question

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.

Answer 1

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]++
  }, {})
}

Answer 2

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

Answer 3

Comments are inline in ///

/// As suggested by other reviewers, splitTextIntoWords or 
/// extractWords would be a more meaningful name.
/// Add default value for parameters, text = ''.
const wordSplit = (text) =>
  /// How about using \w+ to extract words from the text?
  /// It ignores non alphabets and whitespaces.
  /// extractWords = text => text.match(/\w+/g).map(word=>word.toLowerCase());
  text
    /// Declaring regex as a module level constant has slight 
    /// perf benefits as it would only be compiled once.
    .replace(/[.,:;?!]/g, '')
    .split(/\s/)
    .filter(word => word !== '')
    .map(word => word.toLowerCase())

/// Add default value for params, text = ''
const countWords = (text) => {
  const wordCount = {}

  const count = (word) =>
    wordCount[word] ? wordCount[word]++ : wordCount[word] = 1
  }
  /// Holy grail of functional programming!
  /// wordSplit(text).forEach(count)
  /// Wrapping it inside another function is not needed.
  wordSplit(text).forEach(word => count(word))

  return wordCount
}

/// Would getWordFrequencies(text = '', size = 1) be more meaningful?
/// Need to add type and range checks for inputs.
/// `slice` would return incorrect results for negatives. Or you can
/// document that as a `feature`, pull the last frequencies.
/// You would want to export this function.
const wordFrequency = (text = '', topn) =>
  Object
    .entries(countWords(text))
    .sort((a, b) => b[1] - a[1])
    .slice(0, topn)

/// Please add these tests -
/// - "", 0
/// - "", 1
/// - "", -1
/// - ...and more range and type checks.
/// - "      "
/// - `       
///               ` (multi-line, white-space only)
/// - `A quick brown fox
/// -  jumped over the
/// -  lazy dog`  (multi-line)

Overall assessment -

• The code is well written and readable, didn't need explainers.
• Proper usage of const and let, modern ES features.
• Most non-trivial functions are module level, easy testing :).

• You should add tests for the other functions too.

Complexity -

• Considering a string of length M having N disctinct words, top K items.
• Time - O(M + NlogN + K)
• Space - O(M + N + K)