0

so here is an array and i want to do a find and replace for each item in the array and search for the key and replace with the value, but this does not work. it searches for strawberries for 5 times.

Also no idea if i would require a delay with something like this?

var path  = require('path');
var fs    = require('fs');
var cnf   = {
  apples        : 'ill replace this value with this',
  pears         : 'ill replace this value with this',
  bananaas      : 'ill replace this value with this',
  peaches       : 'ill replace this value with this',
  strawberries  : 'ill replace this value with this'
}

for(var k in cnf) {
  fs.readFile(path.resolve('./file-to-search.txt'), 'utf8', function (err,data) {
    if (err)  return console.log(err)
    var searchReplace = data.replace(k, cnf[k])
    fs.writeFile(path.resolve('./file-to-search.txt'), searchReplace, 'utf8', function (err) {
      if (err) return console.log(err)
    });
  });
}
Rob
  • 14,746
  • 28
  • 47
  • 65
Michael Mano
  • 3,339
  • 2
  • 14
  • 35
  • Possible duplicate of [JavaScript closure inside loops – simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – VLAZ Oct 24 '16 at 22:31
  • Thanks for everyones answers, even if some did not do exactly what i wanted they helped me understand what was going on – Michael Mano Oct 24 '16 at 22:52

4 Answers4

1

Rather than reading/writing the file on each search and replace, read the file once, perform each of the search and replaces, then write out the final results.

fs.readFile(path.resolve('./file-to-search.txt'), 'utf8', function (err,data) {
  if (err)  return console.log(err);
  for(var k in cnf) {
    data = data.replace(k, cnf[k]);
  }
  fs.writeFile(path.resolve('./file-to-search.txt'), data, 'utf8', function (err) {
    if (err) return console.log(err);
  });
});
Ouroborus
  • 16,237
  • 4
  • 39
  • 62
  • You are a champ! - Now to just figure out what the hell the difference is so i understand – Michael Mano Oct 24 '16 at 22:51
  • @Unifx Because `readFile` and `writeFile` are asynchronous, your loop makes it possible for the next iteration to read the file before the last iteration gets around to writing to file. (In fact your loop, combined with disk latency and the async read, makes it likely that most of the reads will happen before any writes can start.) When this happens, you can get stale data in later iterations. By limiting the read and the write to once each, you help prevent this situation (though it's still possible if you were to try running the whole thing several times simultaneously). – Ouroborus Oct 25 '16 at 00:04
0

The problem is that fs.readFile runs asyncronously. This means that the variable k goes through all 5 possible values before the first call to fs.readFile has completed. Because k was defined in parent scope, the value changes with the loop.

What you need is a closure.

var path  = require('path');
var fs    = require('fs');
var cnf   = {
  apples        : 'ill replace this value with this',
  pears         : 'ill replace this value with this',
  bananaas      : 'ill replace this value with this',
  peaches       : 'ill replace this value with this',
  strawberries  : 'ill replace this value with this'
}

for(var k in cnf) {
  fs.readFile(path.resolve('./file-to-search.txt'), 'utf8', (function (val) { 
    var v = val; // Closure around the current state of val
    // Return a copy of the function we want to run with the saved state
    return function (err, data) {
        if (err)  return console.log(err)
        var searchReplace = data.replace(v, cnf[v])
        fs.writeFile(path.resolve('./file-to-search.txt'), searchReplace, 'utf8', function (err) {
            if (err) return console.log(err)
        }); // End write
      }); // End Read
   })(k); // End Closure, call with k to get saved state
}

Just a word of warning, when you run into this situation (with a function that runs asynchronously) adding a delay may sometimes appear to work. However, since you can't guarantee the amount of time that the async function will take, you may occasionally get stuff running out of order, which can be REALLY hard to troubleshoot (race condition). Always use callbacks/closures instead of a delay.

vastlysuperiorman
  • 1,694
  • 19
  • 27
  • Sorry to be a pain i tried to spot the issue but its showing test.js:21 }); // End Read ^ SyntaxError: Unexpected token ) – Michael Mano Oct 24 '16 at 22:38
0

The issue is that fs.readFile is aynchronous, which means by the time it's callback has run the loop has completed, so k is referring to the last element, i.e. strawberry.

To resolve this, you need to to wrap this function in a closure:

for (var k in cnf) {
    (function(k) {
        fs.readFile(path.resolve('./file-to-search.txt'), 'utf8', function(err, data) {
            if (err) return console.log(err)
            var searchReplace = data.replace(k, cnf[k])
            fs.writeFile(path.resolve('./file-to-search.txt'), searchReplace, 'utf8', function(err) {
                if (err) return console.log(err)
            });
        });
    })(k);
}

You now have a global k var in the loop and a private k var inside (function(k) {}) which retains the value of k as it occured at that point in the loop.

FuriousD
  • 844
  • 5
  • 16
  • yea this does not work also. ends up replacing strawberries also but messing it up XD - is this because it needs a delay? – Michael Mano Oct 24 '16 at 22:28
  • Can you elaborate? What happens specifically? – FuriousD Oct 24 '16 at 22:30
  • yeah sorry It just ends up doing the following to the files apples pears bananaas peaches strawberries becomes apples pears ill replace this value with this peaches strawberries – Michael Mano Oct 24 '16 at 22:36
0

Sometimes readFileSync() is useful in this sort of situation.

Chris Reynolds
  • 5,453
  • 1
  • 15
  • 12