1
\$\begingroup\$

I've written a GM script to filter some results from Experteer.com results: specifically, many of the rows are "only for Premium members" and don't allow you to click through or see details, so I've written the script to hide those results from cluttering the display.

I'm very much a newbie to JavaScript and cobbled together this script after a lot of googling, so it's quite possibly horrific in several ways, please show me better ways in those cases.

The code follows with some notes on my reasoning:

// ==UserScript==
// @name        ExperteerNoPremium
// @namespace   abiteasier.in
// @description Removes the annoying "only for Premium members" rows from Experteer results
// @include     https://eu.experteer.com/goal/position/*
// @include     https://eu.experteer.com/search/jobs
// @version     0.3
// @grant       none
// ==/UserScript==

var remove_premium_rows = function () {
    var result_rows = document.body.querySelectorAll('.search-result-body');

    for (var i = 0; i < result_rows.length; i++) {
        var row = result_rows[i];

The two different pages included in @include above have the "only for premium" text in different columns, but they both have a td with class="search-result-job-action" which is in both cases two columns before the one we need, hence the selection and double nextElementSibling calls:

        var company_cell = row.querySelector('.search-result-job-action').nextElementSibling.nextElementSibling;

        var premium_only_re = /only\s+for\s+Premium\s+Members/i;
        if (premium_only_re.test(company_cell.textContent)) {
            row.style.display = 'none';
        }
    }
};

remove_premium_rows();

The results page also has a few Ajax controls which update the results table and I would like the pruning to happen on these new results as well. Since Mutation events seem to be discouraged by everyone, I went with a very hacky way of monkeypatching the function that does the Ajax updates itself, adding the remove_premium_rows as a callback:

unsafeWindow.ajaxUpdater = function () {
    new Ajax.Updater('result', '/desire/refine_matches',
            {asynchronous:true, evalScripts:true, method: 'get',
                parameters:Form.serialize($('matches')), 
                onComplete: remove_premium_rows});
}

That's it, the code as above works for my purposes, but I would like to know in what it can be improved to adhere with standard practices.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

If you want to maximize the speed, to iterate big arrays, use

for (var i = 0, l = array.length; i < l; i++)

instead of

for (var i = 0; i < array.length; i++)

Moreover, if you want to get the elements of a class, there's no need to use querySelectorAll, you can use the faster getElementsByClassName (see benchmark)

\$\endgroup\$
1
\$\begingroup\$

It's a minor thing, and it won't make a noticeable difference, but as a matter of principle, you should move this line out of the for loop:

 var premium_only_re = /only\s+for\s+Premium\s+Members/i;

As the value never changes, simply there's no point having it inside the loop.

This statement is hard to read, because multiple attributes are on the same line:

new Ajax.Updater('result', '/desire/refine_matches',
        {asynchronous:true, evalScripts:true, method: 'get',
            parameters:Form.serialize($('matches')), 
            onComplete: remove_premium_rows});

It would be more readable to have one attribute per line, and a space after each :, like this:

new Ajax.Updater('result', '/desire/refine_matches', {
    asynchronous: true, 
    evalScripts: true,
    method: 'get',
    parameters: Form.serialize($('matches')),
    onComplete: remove_premium_rows
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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