6
\$\begingroup\$

The following searches recursively for all the .md files inside a folder. It then stores the text of the files in an array. Finally, it sums their word count using a wordCount() function (from the compromise package).

import nlp from 'compromise'
import fs from 'fs'
import path from 'path'

const filePaths = searchRecursive(`${__dirname}/test`, '.md')

const fileTexts = filePaths.map((filePath: string) => {
    return fs.readFileSync(filePath, 'utf-8')
}) // -> ['one two', 'one two three']

let totalWordCount = 0

fileTexts.forEach((fileText: string) => {
    totalWordCount += nlp(fileText).wordCount()
})

console.log(totalWordCount) // -> 5

function searchRecursive (dir: string, pattern: string) {
    ...
}

I've heard that forEach shouldn't be used in functional programming. How can I modify this code, so it doesn't use forEach?

\$\endgroup\$
  • 7
    \$\begingroup\$ "I've heard that forEach shouldn't be used in functional programming." where was this? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Sep 16 at 18:22
  • 1
    \$\begingroup\$ Ideally, functional programs do not need loops because they can achieve the same intent using recursion, fold, map etc. A general tip from me, 'X should not be used with Y' should never be seen as dogma. IMHO, you should not change your code until you see the reason to (e.g. drawbacks). I don't know javascript but your code looks like it could be changed to 'fold left' if you really want to. \$\endgroup\$ – MaLiN2223 Sep 16 at 18:28
  • 12
    \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ forEach is inherently non-functional, since it returns undefined and thus must perform side-effects to be of any use. To be functional, functions should be pure and without side-effects. \$\endgroup\$ – CertainPerformance Sep 16 at 18:29
  • 2
    \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ: The wording in the question is slightly misleading. It's not that forEach shouldn't be used in functional programming but rather that something like forEach cannot even exist at all in functional programming. forEach returns no value, so it by definition either does nothing at all or purely performs side-effects. Since side-effects don't exist in FP, that means forEach can only exist as a NO-OP. \$\endgroup\$ – Jörg W Mittag Sep 17 at 10:28
8
\$\begingroup\$

Another suggestion: with TypeScript, you only need to note the type of a parameter when TypeScript can't infer it itself. You might find it easier to read and write code when you avoid explicitly typing functions except when necessary for the TS to compile, or when the type of a return value isn't clear at a glance. In other words, if I were you, I'd switch out:

.map((filePath: string) => {

with

.map((filePath) => {

and, if you did have to use forEach somewhere else, switch

fileTexts.forEach((fileText: string) => {

with

fileTexts.forEach((fileText) => {

It cuts down on a bit of syntax noise.


On another note, you're currently waiting for each file to be read in serial due to the readFileSync. If you have 100 files, and it takes 0.1 seconds to read each file, your script will have to run for at least 10 seconds. It would be more elegant to switch out readFileSync with a non-blocking version, and then use Promise.all. Luckily, modern versions of node have fs.promises, which are Promise-based version of the non-blocking functions:

Promise.all(
  filePaths.map(
    filePath => fs.promises.readFile(filePath, 'utf-8')
      // if nlp takes more than an instant to run,
      // make sure to chain it onto the Promise here so it can run ASAP
      // rather than below, which could result in a lot of CPU load at once
      .then(fileText => nlp(fileText).wordCount())
  )
)
  .then((wordCounts) => {
    const totalWordCount = wordCounts.reduce((a, b) => a + b, 0);
    // done
  })
  .catch((err) => {
    // handle errors
  });
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ This is cool, thanks. Does having nlp rely on the FS IO to come back staggered, otherwise it's pretty much no different to having it outside the Promise? Coming from Python this may sound silly, does having nlp in the Promise allow multi-core performance in JS in addition to the benefits to staggered IO? \$\endgroup\$ – Peilonrayz Sep 16 at 18:53
  • 2
    \$\begingroup\$ JS is single-threaded. If you called nlp outside the .map, each nlp call would run in serial. If it takes a bit of processing time, that'd be a bit inelegant - better to run a given nlp as soon as the text is available, to take advantage of (for example) if one file gets its fileText after 0.1 seconds, and another gets its after 0.5 seconds. Unless nlp is expensive, it's only a marginal improvement. \$\endgroup\$ – CertainPerformance Sep 16 at 18:58
  • \$\begingroup\$ Interesting thanks, it seems JavaScript has similar drawbacks as Python does. \$\endgroup\$ – Peilonrayz Sep 16 at 19:03
8
\$\begingroup\$

How to modify this code, so it doesn't use forEach?

One way to achieve this is using Array.prototype.reduce():

const totalWordCount = fileTexts.reduce((big: sum, fileText: string) => {
    return sum + nlp(fileText).wordCount()
}, 0)

Notice that totalWordCount is assigned once so const can be used to avoid accidental re-assignment later.

If fileTexts is only used to calculate totalWordCount then the two loops could be combined into a single loop (e.g. with reduce()).

To learn more about functional techniques try these functional exercises.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks a lot for the answer. Is the function still pure even though it requires a node package to work (compromise)? \$\endgroup\$ – janoChen Sep 16 at 18:34
  • 5
    \$\begingroup\$ After reading a few different definitions about pure functions on various sites I don't see anything about external dependencies (other than things like no variation from I/O devices). I like this SO answer by deceze: ""Having dependencies" plays absolutely no role in defining a pure function. The only thing that matters is whether the function has any side effects and whether its result depends on external state. If a function behaves exactly the same and always produces the same result given the same input, it is pure." \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Sep 16 at 21:16
5
\$\begingroup\$

I'll leave the alternatives of the code to the other answers, but just to make a comment about functional programming:

Functional programming has three core tenets as I see it:

  • Functions are first class citizens (you can pass them as arguments, put them in lists)
  • Functions are deterministic - (for a function call of the same arguments, it always returns the same thing).
  • Functions don't cause side effects.

Now with the second two points it's apparent that in a purely functional context, some things simply are not possible:

  • A function that generates a random number
  • A function that gets the current time
  • A function that fetches some data from a database or API
  • A function that writes data to a file.

So the point here is - even if you're adopting a functional paradigm, at some point, unless your program is just a simple 'sort this list' type program, it likely needs to have side effects and encounter nondeterministic behavior.

In your case, reading from a file is nondeterministic behavior.

So the question is, in a functional paradigm, how do you deal with this?

Some resources I recommend looking at:

https://stackoverflow.com/questions/330371/are-databases-and-functional-programming-at-odds

Basically the suggestion I make is that you isolate your nondeterministic code into functions that just do that non-deterministic behavior. You then pass those functions into the logic functions don't need to know if the function is deterministic or not.

From your example, lets say you have this:

function getFileTexts(filePaths) {
    const fileTexts = filePaths.map((filePath: string) => {
        return fs.readFileSync(filePath, 'utf-8')
    })
    return fileTexts; 
}

So the problem with this is that the fs.readFileSync is nondeterministic.

What you can do pass the 'read file' function in, as an argument.

function getFileTexts(filePaths, readFileFn) {
    const fileTexts = filePaths.map((filePath: string) => {
        return readFileFn(filePath); 
    })
    return fileTexts; 
}

And you would call this with:

const fileTexts = getFileTexts(filePaths, (filePath) =>  fs.readFileSync(filePath, 'utf-8')); 

Now, even though in the real world that function isn't going to deterministic, at least now it's possible for it to be.

That is, in your tests for example, you pass an alternative, purely deterministic function like:

const fileTexts = getFileTexts(filePaths, () => 'foo bar biz'); 
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ Yep, even the console.log(totalWordCount) is non-functional :P At least one side-effect is necessary for a script to be of any use. The best one can do is to minimize them and ensure they're at the very edges of the program. \$\endgroup\$ – CertainPerformance Sep 17 at 14:29

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.