I am not a big fan of JS, but have been forced to use it in this situation for various reasons.

My task: Download a URL to a target file, and update another part of the application with the current percentage.

I've run jslint and jshint over the code, and it seems happy, but I am sure there is still plenty of suboptimal stuff in my code. My code currently looks like this:

Match.prototype.downloadURL = function (url, target, next) {
var tmpFilename = target + ".tmp",
tmpFile = fs.createWriteStream(tmpFilename);
http.get(url, function (resp) {
currentSize = 0,
currentPercentage = -1;
// Write the file
resp.pipe(tmpFile);
resp.on('end', function () {
tmpFile.close();
fs.rename(tmpFilename, target, function (err) {
if (err !== null) {
return next(err);
}
next(null, target);
});
});
// Update when we hit a new percentage
resp.on('data', function (chunk) {
currentSize += chunk.length;
var percentage = parseInt(currentSize / fullSize * 100, 10);
if (percentage === currentPercentage) {
return;
}
currentPercentage = percentage;
});
});
};


Problems I can see:

1. updateDownloadingPercentage and registerDownloadFinished are networked operations; I should probably be doing something to make sure these things arrive in order
2. I have a feeling I should be checking for errors a lot more; createWriteStream can fail, so where do I get its error message from.

Again, I only ask because I am a very poor JavaScript coder. Please enlighten me as to any way I can improve this snippet.

• You need a resp.on('error', function () {} to deal with http errors
• You need a tmpFile.on('error', function () {} to deal with fs/stream errors
• tmpFile is an unfortunate name, i would call it maybe tmpStream or tmpFileStream
• resp should be res ( standard in Node ), or response, it bugs me every time I read the code ;)