Code from the "Dependency Injection Basics" episode of Fun Fun Function

episode.js

const assert = require('assert')

function getAnimals(fetch, id) {
  return fetch('http://api.animalfarmgame.com/animals/' + id)
    .then(response => response.json())
    .then(data => data.results[0])
}

describe('getAnimals', () => {
  it('calls fetch with the correct url', () => {
    const fakeFetch = url => {
      assert(
        url ===
        'http://api.animalfarmgame.com/animals/123'
      )
      return new Promise(function(resolve) {

      })
    }
    getAnimals(fakeFetch, 123)
  })

  it('parses the response of fetch correctly', (done) => {
    const fakeFetch = () => {
      return Promise.resolve({
        json: () => Promise.resolve({
          results: [
            { name: 'fluffykins' }
          ]
        })
      })
    }
    getAnimals(fakeFetch, 12345)
      .then(result => {
        assert(result.name === 'fluffykins')
        done()
      })
  })
})

@mpj and @koresar:

If i understand this code correctly:

import boom from 'boom';
import {userDB, folderDB} from './db';
import {userAccess, fileStorage} from './somewhere';

function storeDocument(userID, folderLocation, fileStream) {
  return Promise.all([
    userDB.findOne(userID),
    folderDB.findOne(folderLocation)
  ])
  .then(([user, folder]) => userAccess.canWrite(user, folder))
  .then(allowed => {
    if (!allowed) return Promise.reject(boom.forbidden());
    return fileStorage.save(fileStream, {user, folder});
  });
}

then boom.forbidden() is more or less a constant which we don't need to replace with a stub, we are actually very interested in exactly this result, right?

Other observations about the code is that we can use composition of the Promise Monad and write alot of it without needing Promises at all:

const throwForbiddenOnFalse = value => {
  if (value)
  {
    throw boom.forbidden();
  }
};

This part would be super simple to test...

Other things... that the original code does in the same function is fetching data from databases, looking up Access policies, and writing the data to a stream.

checking the access:

const checkAccess = userAccess => (user, folder) => userAccess.canWrite(user, folder)
    .then(throwForbiddenOnFalse)

I have used currying here to help with configuration, so that the user of the function at runtime don't need to know which userAccess is being used.

saving the actual file:

const saveFileStream = fileStorage => (fileStream, user, folder)  => fileStorage.save(fileStream, {user, folder});

combining them:

const checkAccsessAndSaveStream = (checkAccess, saveFileStream) => (user, folder, fileStream) =>
    checkAccess(user, folder).then(() => saveFileStream(fileStream, user, folder));

and the final product:

const fetchUserAndFolderThenStoreDocument = (userDB, folderDB, checkAccessAndSaveStream) =>
  (userID, folderLocation, fileStream) =>
    Promise.all([userDB.findOne(userID), folderDB.findOne(folderLocation)])
      .then(([user, folder]) => checkAccessAndSaveStream(user, folder, fileStream));

so the factory would look something like this:

const storeDocument = fetchUserAndFolderThenStoreDocument(
  userDB,
  folderDB,
  checkAccsessAndSaveStream(checkAccess, saveFileStream(fileStorage))
);

some of the interfaces feels klunky, and they could be a written abit better with reader monads and cofunctors, depending on how the two db objects are related.

Also note that the real factory would be simpler since it would be using other factories also that the tests would be simpler since they would only care about the dependencies one level down.

So the real benefit here is that the checkAccsessAndSaveStream function don't need to know about the databases or the fileStorage. You could say that these dependensies juped over the parents of the user and made them simpler.

I also have the feeling that there would be even more code that could have been written without needing to know that it was async.

what do you think?

I did some more thingking and this fileStorage.save(fileStream, {user, folder}) strikes me as a bit odd. why do the filesStorage need the user and folder. Is it somehow part of the location that the fileStreamwill bes saved at?

Also there is some asymmetri here. storeDocument gets raw data as arguments, except for the stream object. but the access method userAccess.canWrite(user, folder) takes full-blown object. there might be some answers hidden in this odd relationship.