0

I have a function which does something async like saving to database. Want a mechanism that first inserts the row and the next insertion should occur only when the first insert operation has finished.

Here is what I have tried and it somewhat works.

var interval = true;

function insert() {
    model.save(function () {
        interval = true;
    })
}
foreach(row, function (key, val) {
    var interval1 = setInterval(function () {
        if (interval) {
            insert();
            interval = false;
            clearInterval(interval1);
        }
    }, 100)
})

Is it the correct approach of doing this? Please shed some light about my understanding of timers in javascript.

beNerd
  • 3,314
  • 6
  • 54
  • 92
  • 2
    Eeek, this is horrible. You don't want to poll for completion. You want to use the completion handler to actually start the next iteration. You can't use a foreach loop with async operations like this. Write a function that executions the next iteration. Call that function on each completion. Use however many variables you need to keep the state for each next iteration. – jfriend00 Mar 23 '14 at 16:39
  • how do i do that? I have a json like structure that i need to loop through :( – beNerd Mar 23 '14 at 16:40
  • Show us what the `row` data structure looks like. Also, does `insert()` really take no arguments? – jfriend00 Mar 23 '14 at 16:42
  • Its actually a .onRecord callback from a csv reader library that reads each row and returns json – beNerd Mar 23 '14 at 16:44
  • I put an example of how to iterate progressively without polling into my answer below. – jfriend00 Mar 23 '14 at 16:59

3 Answers3

3

No, you should not be creating timers to poll for when something is done. That's probably the worst way you can do it. What you want to do is to explicitly start the next iteration each time the previous one finishes.

Here's the general idea for how you do this without polling. The idea is that you need to create a function that can be called successive times and each time it's called, it will perform the next iteration. You can then call that function from the completion handler of your async operation. Since you don't have a nice convenient foreach loop to control the iteration, you then have to figure out what state variables you need to keep track of to guide each iteration. If your data is an array, all you need is the index into the array.

function insertAll(rows) {

    // I'm assuming rows is an array of row items

    // index to keep track of where we are in the iteration
    var rowIndex = 0;

    function insert() {
        // keep going as long as we have more rows to process
        if (rowIndex < rows.length) {
            // get rows[rowIndex] data and do whatever you need to do with it

            // increment our rowIndex counter for the next iteration
            ++rowIndex;
            // save and when done, call the next insert
            model.save(insert)
        }
    }
    // start the first iteration
    insert();
}

If you don't have your data in an array that is easy to step through one at a time this way, then you can either fetch each next iteration of the data when needed (stopping when there is no more data) or you can collect all the data into an array before you start the operation and use the collected array.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • This approach does makes sense. The reader has two callbacks. Onrecord and onEnd. So at each onRecord, I push the row object in an array, and on the onEnd callback i use the method described by you and @felix king. Correct? – beNerd Mar 23 '14 at 17:02
  • thanks. A quick question: ++i and i++ would make any difference? I have seen many people using ++i but i generally use i++ – beNerd Mar 23 '14 at 17:20
  • @beNerd - See http://stackoverflow.com/questions/3469885/somevariable-vs-somevariable-in-javascript for discussion of `++` before and after. In this case, it makes no difference which you use because it's not involved in an expression. – jfriend00 Mar 23 '14 at 17:37
1

No, this is absolutely not the right way to do this. Lets assume that row contains 10 values, then you are creating 10 independent timers which continuously run and check whether they can insert. And it's not even guaranteed that they are executed in the order they are created.

As jfriend00 already mentioned, you should omit the "loop" and make use of the completion callback of the save operation. Something like this:

var rows = [...];

function insert(rows, index) {
    index = index || 0;
    var current_element = rows[index];

    model.save(function() {
        if (index < rows.length - 1) {
            insert(rows, index + 1);
        }
    });
}

insert(rows);

Notice how the function calls itself (somehow) after the save operation is complete, increasing the index so the next element in the array is "saved".

Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
  • thanks. Its actually not a loop. Its actually a .onRecord callback from a csv reader library that reads each row and returns json. For the purpose of illustration, I used the loop in the code. – beNerd Mar 23 '14 at 16:48
  • Then I don't think you even have a problem. Why don't you just use something like `reader.onRecord(function(row) { saveRow(row); });`? – Felix Kling Mar 23 '14 at 16:51
  • Used that but the problem is that I want to initiate the next record only when the first one has finished. Duplicacy check needs to be done. For example, In the first iteration, it starts the insertion. Now if the second insertion has the duplicate value, It won't detect that because first one hasn't yet finished? Hope you got my point. – beNerd Mar 23 '14 at 16:55
  • I see. Then you have to store all records in a an array first and do as I showed. – Felix Kling Mar 23 '14 at 16:56
  • yeah exactly what i was thinking. So this is the only way to do this? – beNerd Mar 23 '14 at 16:57
  • There are other possible ways, but it's rather unlikely that the reader you use supports it, because it's probably not such a common use case. – Felix Kling Mar 23 '14 at 16:59
  • This approach does makes sense. The reader has two callbacks. Onrecord and onEnd. So at each onRecord, I push the row object in an array, and on the onEnd callback i use the method described by you and @felix king. Correct? – beNerd Mar 23 '14 at 17:03
0

I would use a library that handles async stuff such as async.js

BTW it seems like your model.save methods takes a callback, which you can use directly to call the insert method. And if the insert function is one you have made by yourself, and not a part of some bigger framework, I will suggest to re-write it and make take a callback as parameter, and use that instead of using setInterval for checking when async work is done.

thetrompf
  • 430
  • 3
  • 11