How to deduplicate code in a callback?.md

Consider the following function:

function isContainingDirectory(path, callback) {
  const directoryName = getDirectoryName(path),
        numTotalChecks = 2;

  let numChecksDone = 0;
      callbackHasBeenCalled = false;

  fs.access(
    `${path}/${directoryName}`,
    err => {
      if (callbackHasBeenCalled) return;
      if (err) {
        callback(null, false);
        callbackHasBeenCalled = true;
      } else {
        if (++numChecksDone === numTotalChecks) callback(null, true);
      }
    }
  );

  fs.access(
    `${path}/gh-pages`,
    err => {
      if (callbackHasBeenCalled) return;
      if (err) {
        callback(null, false);
        callbackHasBeenCalled = true;
      } else {
        if (++numChecksDone === numTotalChecks) callback(null, true);
      }
    }
  );
}

The same operations are being done for two different values. We can shorten it to:

function isContainingDirectory(path, callback) {
  const directoryName = getDirectoryName(path);

  let numChecksTotal = 0,
      numChecksDone = 0,
      callbackHasBeenCalled = false;

  checkIfFileExists(`${path}/${directoryName}`);
  checkIfFileExists(`${path}/gh-pages`);

  function checkIfFileExists(directory) {
    fs.access(
      directory,
      err => {
        if (callbackHasBeenCalled) return;
        if (err) {
          callback(null, false);
          callbackHasBeenCalled = true;
        } else {
          if (++numChecksDone === numChecksTotal) callback(null, true);
        }
      }
    );
    numChecksTotal++;
  }
}

Even better:

function isContainingDirectory(path, callback) {
  const directoryName = getDirectoryName(path),
        directoriesToCheck = [
          `${path}/${directoryName}`,
          `${path}/gh-pages`,
        ],
        numChecksTotal = directoriesToCheck.length;

  let numChecksDone = 0,
      callbackHasBeenCalled = false;

  directoriesToCheck.forEach(
    directory => {checkIfFileExists(directory)}
  );

  function checkIfFileExists(directory) {
    fs.access(
      directory,
      err => {
        if (callbackHasBeenCalled) return;
        if (err) {
          callback(null, false);
          callbackHasBeenCalled = true;
        } else {
          if (++numChecksDone === numChecksTotal) callback(null, true);
        }
      }
    );
  }
}

If we don't want checkIfFileExists to be a closure, we can change the code to following. I'm not sure if there are any pros or cons of the code being a closure or not:

function isContainingDirectory(path, callback) {
  const directoryName = getDirectoryName(path),
        directoriesToCheck = [
          `${path}/${directoryName}`,
          `${path}/gh-pages`,
        ],
        state = {
          numChecksTotal: directoriesToCheck.length,
          numChecksDone = 0,
          callbackHasBeenCalled = false,
        };

  directoriesToCheck.forEach(
    directory => {checkIfFileExists(directory, state)}
  );

  function checkIfFileExists(directory, state) {
    fs.access(
      directory,
      err => {
        if (state.callbackHasBeenCalled) return;
        if (err) {
          callback(null, false);
          state.callbackHasBeenCalled = true;
        } else {
          if (++state.numChecksDone === state.numChecksTotal) {
            callback(null, true);
          }
        }
      }
    );
  }
}

Note that pedantically, it is still a closure, because it is referencing the fs variable, which holds the result of the require('fs') function call.