HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Promises turning into pyramids

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
intoturningpromisespyramids

Problem

I'm using node-ftp to download some files from an FTP server; due to nodeback difficulty and nested calls I'm promisified some of the calls, hoping that this will help get me out of pyramid code hell, but I think I'm still mis-applying something.

Before I get to my code pasted below, conceptually the pseudo-code of what I'm trying to do is:

foreach directory in a list:
    list files in directory
    filter to only files of interest
    download those files, save them to /tmp/whatever.


The objective is to end with a list of files downloaded and local paths.

I have code that works, but it's god-awful ugly and is not living up to the "promise of promises". How can I re-arrange this to be more like the pseudo code above, and aggregate the results? Part of the trouble is that the "list files in directory" is a promise, and each download returns a promise of a data stream. So unless I do this smartly, it's a 3-level nested array of promises that all have to be resolved, with the very bottom level promise results the intended return value.

```
//Main handler: when the connection is ready, start doing stuff.
c.on('ready', function() {
config.clientCheckPaths
.map(function (lookInPath) {
console.log('Examining ' + lookInPath);

// Go get directory contents.
listAsync(lookInPath)
.then(function (files) {
console.log('Files: ' + files.map(function (i) { return i.name; }));

var wantedFiles = files.filter(function (item) {
return config.isFileWanted(item.name);
});

// Filter it down to only the files we want.
var arrayOfDownloadPromises = wantedFiles.map(function (x) {
// For each file, go download it.
return new Promise(function (resolve, reject) {
var fullPath = lookInPath + '/' + x.name;

console.log("Wanted file: " + lookInPath + '/' + x.name + ", " + Object.keys(x))

Solution

Here's a quick attempt to clean it up:

c.on('ready', function() {
  return Promise.all(config.clientCheckPaths.map(function (lookInPath) {    
    console.log('Examining ' + lookInPath);
    // Go get directory contents.
    return listAsync(lookInPath).then(function(files){
      return files.map(function(i){
        i.lookInPath = lookInPath;
        return i;
      });
    })
  }))
  .then(flatten)
  .then(function (files) {
    console.log('Files: ' + files.map(function (i) { return i.name; }));
    // Filter it down to only the files we want.
    return Promise.all(
      files.filter(function (item) {
        return config.isFileWanted(item.name);
      }).map(downloadWantedFiles)
    );
  });
});

function downloadWantedFiles(x) {
  // For each file, go download it.
  var fullPath = x.lookInPath + '/' + x.name;
  console.log("Wanted file: " + x.lookInPath + '/' + x.name + ", " + Object.keys(x));
  return getAsync(fullPath)
    .then(writeLocalFile(fullPath))
    .catch(couldNotDownload(fullPath))
  });
}

function writeLocalFile(fullPath) {
  return function (stream) {
    // With the stream, name it something reasonable on the disk.
    var localFileName = '/tmp/' + fullPath.replace(/[^A-Za-z0-9\.]/g, '_');
    console.log("Writing stream to " + localFileName);
    var write = stream.pipe(fs.createWriteStream(localFileName));
    // Only resolve once the local file has been written to disk
    return new Promise(function(resolve, reject){
      write.on('end', resolve.bind(null, localFileName));
      write.on('error', reject);
    });
  }
}

function couldNotDownload(fullPath) {
  return function (err) {
    var msg = 'Getting file ' + fullPath + ' failed with ' + err;
    console.error(msg);
    return Promise.reject(err);
  }
}

function flatten(arr) {
  return arr.reduce(function (memo, val) {
    var args = Array.isArray(val)
      ? flatten(val)
      : [val];
    memo.push.apply(memo, args);
    return memo;
  }, []);
}

Code Snippets

c.on('ready', function() {
  return Promise.all(config.clientCheckPaths.map(function (lookInPath) {    
    console.log('Examining ' + lookInPath);
    // Go get directory contents.
    return listAsync(lookInPath).then(function(files){
      return files.map(function(i){
        i.lookInPath = lookInPath;
        return i;
      });
    })
  }))
  .then(flatten)
  .then(function (files) {
    console.log('Files: ' + files.map(function (i) { return i.name; }));
    // Filter it down to only the files we want.
    return Promise.all(
      files.filter(function (item) {
        return config.isFileWanted(item.name);
      }).map(downloadWantedFiles)
    );
  });
});

function downloadWantedFiles(x) {
  // For each file, go download it.
  var fullPath = x.lookInPath + '/' + x.name;
  console.log("Wanted file: " + x.lookInPath + '/' + x.name + ", " + Object.keys(x));
  return getAsync(fullPath)
    .then(writeLocalFile(fullPath))
    .catch(couldNotDownload(fullPath))
  });
}

function writeLocalFile(fullPath) {
  return function (stream) {
    // With the stream, name it something reasonable on the disk.
    var localFileName = '/tmp/' + fullPath.replace(/[^A-Za-z0-9\.]/g, '_');
    console.log("Writing stream to " + localFileName);
    var write = stream.pipe(fs.createWriteStream(localFileName));
    // Only resolve once the local file has been written to disk
    return new Promise(function(resolve, reject){
      write.on('end', resolve.bind(null, localFileName));
      write.on('error', reject);
    });
  }
}

function couldNotDownload(fullPath) {
  return function (err) {
    var msg = 'Getting file ' + fullPath + ' failed with ' + err;
    console.error(msg);
    return Promise.reject(err);
  }
}

function flatten(arr) {
  return arr.reduce(function (memo, val) {
    var args = Array.isArray(val)
      ? flatten(val)
      : [val];
    memo.push.apply(memo, args);
    return memo;
  }, []);
}

Context

StackExchange Code Review Q#113668, answer score: 3

Revisions (0)

No revisions yet.