patternjavascriptMinor
Promises turning into pyramids
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:
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))
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.