current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I want to read multiple files simultaneously and apply the same function on data.

This code works correctly, but can someone please suggest any better way of writing this?

function fileread(path,stats,callback){
   var buffer = new Buffer(stats.size);
   fs.readFile(path, buffer, function(err, data){
      if(err){
         throw err;
      }

      callback(null, data);
   });
}

function testAsync() {
    var files = ['Statistics_Schema.json','seriesData'];
    async.map(files, fs.stat, function(err, stats){
       var i=0;
       async.map(files, function(path, callback){
          fileread(path, stats[i++], callback);
          },function(err, data){
                for(var j=0; j < data.length; j++){
                 console.log(data[j].toString());
                }
           });
    });
}
share|improve this question
    
Use promise pattern. Recommend to read: strongloop.com/strongblog/… –  Junle Li Aug 12 at 5:19

2 Answers 2

up vote 2 down vote accepted

I believe you can shorten it to just

function testAsync() {
    var files = ['Statistics_Schema.json', 'seriesData'];
    async.map(files, fs.readFile, function (err, data) {
        for(var i = 0, l = data.length ; i < l ; i++) {
            console.log( data[i].toString() );
        }
    });
}

Right now, you're first calling fs.stat on each file, only so you can create a buffer in your fileread method.

But fs.readFile does not take a buffer argument. It takes an options object (and there's no buffer option).

The Buffer object you're passing is an object, though, so Node doesn't complain. It just doesn't find any relevant fileRead options in that object either, so it's basically ignored.

So you don't need the buffer, which means you don't need fs.stat, which means you can skip almost all of your code.

share|improve this answer
up vote 1 down vote

Why not using fs.readFile? I just tested this code:

var async = require('async'),
    fs = require('fs'),
    files = ['file1.json', 'file2.json'];

async.map(files, fs.readFile, function(err, files) {
    if(err) {
        throw err;
    }

    files.forEach(console.log);
});

If you wanna be able to read the file contents then replace the last line with

files.forEach(function(file) {
    console.log(file.toString());
});
share|improve this answer
    
He is using fs.readFile in fileread with a buffering option. –  konijn Aug 13 at 13:33
1  
Hi @konjin - I thought that with both codes he would achieve the same results, then I googled about the difference in passing a buffer to readFileand could find nothing on the docs. Can you explain the difference? –  renatoargh Aug 13 at 13:45
    
Interesting, it seems you are correct, passing buffer to readFile does not make any sense. –  konijn Aug 13 at 13:49
    
Also, see this: codereview.stackexchange.com/a/59783/14625 –  konijn Aug 13 at 13:50
1  
At the time I answered @Flambino's answer wasn't visible to me, I believe its due to the migration from SO. Pretty much the same answer. –  renatoargh Aug 13 at 15:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.