Reading genetic data in VCF and Tabix formats using an asynchronous library

Question

I'm working with an open-source library for processing and parsing genetic data in VCF and Tabix formats. It contains functions and classes that make it easy to read an index file (a Tabix) and load corresponding data from a compressed VCF.

It works asynchronously (unsurprisingly), however, and I'm not sure of the best way to structure my code. I'm looking to make multiple calls to the getRecords() method—since I need to retrieve multiple discontinuous segments of data; because this is asynchronous, it seems like I also have to instantiate a new file reading object (here known as readBinaryVCF but roughly analogous to HTML's FileReader). Both the file reading object's constructor and the getRecords() function use callbacks.

I'm not really experienced in JavaScript or asynchronous design patterns; I'm hoping it doesn't have to be this convoluted. Other solutions that I tried, though, like recursing or making multiple calls to getRecords() on the same object, were not as flexible or as robust—or just flat out didn't work.

The code sample below is the first iteration I've gotten to work. That being said, please tear it apart. I suspect there're many improvements to be made.

EDIT: I'll leave this code alone as my first draft, but please look at the minor changes I've made in an answer below. I'm still hoping for help, preferably on top of my revisions.

var regions = [ 
    [10, 10000, 100000], 
    [12, 10000, 100000], 
    [14, 10000, 100000], 
    [18, 10000, 100000]
];


var d = [];
var readers = [];

for (var a = 0; a < regions.length; a++) {
    d.push([]);
    readers.push(new readBinaryVCF(tabix, vcf, function(x) {x = x;})); 
    //Should I just initialize these to null instead? 
}

regions.forEach(function(item, i) {

    readers[i] = new readBinaryVCF(tabix, vcf, function(tabixReader) {

        rbvcf = readers[i];

        var index = 0; 
        for (key in tabixReader.idxContent.namehash) {
            if (key == regions[i][0]) {
                index = tabixReader.idxContent.namehash[key];
            }
        }

        rbvcf.getRecords(index, regions[i][1], regions[i][2], function(data) {
            d[i] = data; 

            if (i == regions.length - 1) {
                var finalText = "";

                for (var j = 0; j < d.length; j++) {
                    for (var k = 0; k < d[j].length; k++) {
                        finalText = finalText + d[j][k] + "<br />";
                    }
                }

                $(".text").html(finalText);
            }
        });
    });
});

Answer 1

A few painfully obvious edits of my own:

The frivolous for-loop:

for (var a = 0; a < regions.length; a++) {
    d.push([]);
    readers.push(new readBinaryVCF(tabix, vcf, function(x) {x = x;})); 
}

Why, why did I think this was a good idea? It's entirely unnecessary.

Omitted.

Callback-waiting:

if (i == regions.length - 1) {

   var finalText = "";

   for (var j = 0; j < d.length; j++) {
       for (var k = 0; k < d[j].length; k++) {
           finalText = finalText + d[j][k] + "<br />";
       }
   }

   $(".text").html(finalText);
}

I thought I could wait until the "last" callback had ended before parsing my data, not realizing that—because they're, again, callbacks—they won't finish in order. This answer inspired me to use a manual counter to determine when they've all finished.

Revised code:

var d = []; 
var readers = []; 
var callbacksRemaining = regions.length; 

regions.forEach(function(item, i) {

    readers[i] = new readBinaryVCF(tabix, vcf, function(tabixReader) {

        reader = readers[i];

        var index = 0; 

        for (key in tabixReader.idxContent.namehash) {
            if (key == regions[i][0]) {
                index = tabixReader.idxContent.namehash[key];
            }
        }

        reader.getRecords(index, regions[i][1], regions[i][2], function(data) {; 

            d[i] = data; 
            callbacksRemaining--; 

            if (callbacksRemaining <= 0) {
                processData(d);
            }
        });
    });
});

Summary:

I've made these few minor edits, but my code still isn't running as fast as I'd like. I'm sure there are many more changes I could and should make—I'm still hoping for advice!

Answer 2

I was posting about the same answer you did, but I inspired this review on your answer, not your question.

    var regions = [
        [10, 10000, 100000],
        [12, 10000, 100000],
        [14, 10000, 100000],
        [18, 10000, 100000]
    ];


    var d = [];
    var readers = [];
    var callbacksRemaining = regions.length;

    regions.forEach(function(item, i) {
        // strip the function from your forEach loop for code control.
        readers[i] = new readBinaryVCF(tabix, vcf, nameThisProperly);

        var nameThisProperly = function(tabixReader) {

            // add var here
            // you're not in strict mode so this gets pushed to the global scope
            // in strict mode you get errors.
            var reader = readers[i];

            // you can use reversed psychology for this loop:
            var index = 0;
            for (var key in tabixReader.idxContent.namehash) {
                if (key == regions[i][0]) {
                    index = tabixReader.idxContent.namehash[key];
                }
            }
            // example:
            var index = tabixReader.idxContent.namehash[regions[i][0]];
            // obviously untested, but something like this should work.


            // using index after the for loop seems weird. I would rename this variable, if it is only used to
            // work with the last element of the loop. Make it a good one ;) Now its like you would
            // run this method every time the for loop is called, but it is not.
            reader.getRecords(index, regions[i][1], regions[i][2], checkForCallback);
        };

        // strip this method for readability
        var checkForCallback = function(data) {
            d[i] = data;
            callbacksRemaining--;

            if (callbacksRemaining <= 0) {
                processData(d);
            }
        };
        // naming your functions like this makes it easy to know what's happening.
    });