Optimize JQuery/JavaScript Link Checker

Question

Below is a script I've written that uses AJAX to check if all links on the page are alive or dead. Could this be modified to be more efficient in any way? Thank you very much.

$(document).ready(function() {
    var liveLinks = [];
    var deadLinks = [];

    function updateLive() {
        $live = Number($('#liveLinkCount').text()) + 1;
        $('#liveLinkCount').text($live);
    }

    function updateDead() {
        $dead = Number($('#deadLinkCount').text()) + 1;
        $('#deadLinkCount').text($dead);
    }

    $('body').prepend("<div id='linkChecker'><span id='hideChecker'>Hide</span><button id='checkLinks'>Check Links</button><br /><span>Live: </span><span id='liveLinkCount' style='color:green;'>0</span><span> - Dead: </span><span id='deadLinkCount' style='color:red;'>0</span></div>");
    $('#linkChecker').css({
        "width": "130px",
        "border": "1px solid black",
        "text-align": "center"
    });
    $('#hideChecker').css({
        "text-decoration": "underline",
        "float": "right",
        "padding": "1px",
        "cursor": "pointer"
    });
    $('body').on('click', '#checkLinks', function() {
        $('#linkChecker').append("<br /><button id='logReport'>Log Report</button>");
        $('a').each(function(i) {
            var self;
            self = this;
            $.ajax({
                type: "post",
                url: $(self).prop('href'),
                success: function() {
                    liveLinks.push($(self).prop('href'));
                    updateLive();
                    $(self).css({
                        "color": "green"
                    });
                },
                error: function() {
                    deadLinks.push($(self).prop('href'));
                    updateDead();
                    $(self).css({
                        "font-weight": "bold",
                        "color": "red"
                    });
                }
            });
        });
    });
    $('body').on('click', '#logReport', function() {
        console.log(new Array(24 + 1).join('\n'));
        console.log("Live Links:");
        for (var i in liveLinks) {
            console.log("  " + liveLinks[i]);
        }
        console.log("Dead Links:");
        for (var i in deadLinks) {
            console.log("  " + deadLinks[i]);
        }
    });
    $('body').on('click', '#hideChecker', function() {
        $('#linkChecker').fadeOut();
    });
});

Answer 1

First of all, jQuery's error callback gets invoked on network errors, parser errors, or 4xx/5xx HTTP status codes.

The status codes do not necessarily mean that a link is dead. At least there's a server at the other end. Of course, 404 Not found or 410 Gone are pretty unambiguous, but something like 403 Forbidden could mean that you just aren't allowed to see a certain page - but others might be. Similarly, a 405 Method Not Allowed would mean that your GET request was rejected, because the server expects a POST or something else (normally links mean GET of course, but the page might have some JS that turns a regular link into a POST form submission).

Parser errors could also trip up your system. For instance a link to some malformed XML might cause an error because jQuery might try to parse it, but there is technically some content there, so the link's not exactly dead.

In other words, don't rely on error to always mean "dead link".

Second: Keep a list of URLs you've already checked, and avoid checking the same page twice. You can also do try making some informed guesses. E.g. if a link to "example.com/foo" gets a network error, there a good chance all links to example.com are down. It's not guaranteed of course, but worth considering.

Third: You should ignore links that start with # since they are local to the page you're already on (or their behavior in defined with javascript, in which case you still don't want to check them). You'll also want to avoid those that don't have a href attribute at all, or better yet, report it as a different kind of issue (it's rare and not valid to have such links, but it happens).

Depending on your needs, you could also have an option to skip all "local" links, and only check those that to other sites. E.g. I'm pretty confident that the links to other CodeReview questions on this page are all good, so I might want to limit the checking to external sites only. Or vice-versa.

Fourth: console.log(new Array(24 + 1).join('\n'));
What? I'm pretty sure 24 + 1 will always be 25, so that's one definite optimization you can do. But what do you want with 25 blank lines anyway? Just curious.

Fifth: This is debatable, but you may want to set cache: false in the ajax options. It will slow things down, but you'll also be sure you're checking "for real" and not just getting cached pages.

Sixth: I'd advise you to keep the UI code and the actual checking "engine" more separate. Keeps the code cleaner.

Answer 2

Firstly, for...in is used to enumerate over object properties, not to iterate through arrays. You should use a traditional for loop for this.

Secondly, you could probably refine your console logging portion a bit. It may not have any impact to application performance, but it would make your console look prettier.

$('body').on('click', '#logReport', function() {
    var i, j, k, l;

    console.group("Live Links:");
    for (i=0, j=liveLinks.length; i<j; i++) {
        console.log(liveLinks[i]);
    }
    console.groupEnd();
    console.group("Dead Links:");
    for (k=0, l=deadLinks.length; k<l; k++) {
        console.log(deadLinks[k]);
    }
    console.groupEnd();
});

As far as I know, this works for both Google Chrome and Mozilla Firefox (FireBug). See here for more information.