Two similar MongoDB-querying functions to find distinct and non-distinct documents

Question

I'd like to remove some of the repeated code in my mongo data access layer. The code is all the same except for these key areas:

1. Function signature parameters differ between collectionFind and collectionDistinct though they both have a callback cb.
2. The line below // HERE IS THE LINE THAT VARIES

I suspect I can use a closure to pass in cb to the (innerErr, result) callback but that only solves part of the problem so was curious about other ideas.

  collectionFind: function collectionFind(collectionName, query, fields, options, cb) {
    var url = getUrl();
    if (!url) {
      logger.error('No mongodb connection set');
    } else {
      getConnection(url, (err, db) => {
        if (err) {
          logger.error(`Cannot connect to mongo: ${err}`);
          clearConnection();
        } else {
          // HERE IS THE LINE THAT VARIES
          find(db, collectionName, query, fields, options, (innerErr, result) => {
            if (innerErr) {
              clearConnection();
              cb(innerErr);
            } else {
              cb(null, result);
            }
          });
        }
      });
    }
  },
  collectionDistinct: function collectionDistinct(collectionName, field, filter, cb) {
    let url = getUrl();
    if (!url) {
      logger.error('No mongodb connection set');
    } else {
      getConnection(url, (err, db) => {
        if (err) {
          logger.error(`Cannot connect to mongo: ${err}`);
        } else {
          // HERE IS THE LINE THAT VARIES
          distinctDocuments(db, collectionName, field, filter, (innerErr, result) => {
            if (innerErr) {
              clearConnection();
              cb(innerErr);
            } else {
              cb(null, result);
            }
          });
        }
      });
    }
  }

Answer 1

You could return early on errors. This way, you don't nest as much.

I'm not sure if getConnection, find and distinctDocuments are custom functions or library functions based on the example. But if you are able to update their implementation or swap it out for another library, use Promises. That way you avoid callback hell.

Also, I really hate functions that do too much. Some developers tend to just put multiple similar functionalities in one function and distinguish them via an argument. But when more functionality is added or when the functionalities diverge, the implementation becomes monolithic and hard to refactor. So I suggest that until a third or fourth function with the same flow comes in, I would keep it as is.

If the implementation used promises, it would be as easy as:

collectionFind: function collectionFind(collectionName, query, fields, options) {
  const url = getUrl();

  if (!url) return logger.error('No mongodb connection set');

  return getConnection(url)
    .then(db => find(db, collectionName, query, fields, options))
    .catch(err => {
      logger.error(err);
      clearConnection();
      throw err; // rethrow
    });
}
collectionDistinct: function collectionDistinct(collectionName, field, filter) {
  const url = getUrl();

  if(!url) return logger.error('No mongodb connection set');

  return getConnection(url)
    .then(db => distinctDocuments(db, collectionName, field, filter))
    .catch(err => {
      logger.error(err);
      clearConnection();
      throw err; // rethrow
    });
}

// Usage
ns.collectionFind(...).then(result => {

}, err => {

});

ns.collectionDistinct(...).then(result => {

}, err => {

});