3
\$\begingroup\$

This is my code:

function search() {
    $("#myInput").keyup(function() {
        var value = this.value.toLowerCase();

        $("table").find("tr").each(function(index) {
            if (index === 0) return;
            var id = $(this).find("td").first().text().toLowerCase();
            $(this).toggle(id.indexOf(value) !== -1);
        });
    });
}

function searchFilt(value) {
    $("table").find("tr").each(function(index) {
        if (index === 0) return;
        var id = $(this).find("td").first().text();
        $(this).toggle(id.indexOf(value) !== -1);
    });
};

However I have used:

$("table").find("tr").each(function(index) {
            if (index === 0) return;
            var id = $(this).find("td").first().text().toLowerCase();
            $(this).toggle(id.indexOf(value) !== -1);
        });
    });

twice, and I am sure there would be a more effective way to do this. However I am not sure how to go about doing so - any advice?

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

In the function searchFilt you are using a parameter value, similar to the block of code in the search function. You can just call the searchFilt function from within the search function and pass the variable value:

function search() {
    $("#myInput").keyup(function() {
        var value = this.value.toLowerCase();
        searchFilt(value);
    });
}

function searchFilt(value) {
    $("table").find("tr").each(function(index) {
        if (index === 0) return;
        var id = $(this).find("td").first().text();
        $(this).toggle(id.indexOf(value) !== -1);
    });
}

Note: you don't exactly use the code twice, inside the search function you have .toLowerCase() at the end of the line where you get the id.

\$\endgroup\$
  • 1
    \$\begingroup\$ value is used in $(this).toggle(id.indexOf(value) !== -1); (the last line of searchFilt) so you can update your answer to reflect that \$\endgroup\$ – 3Dos Dec 19 '17 at 13:36
  • \$\begingroup\$ @3Dos Still baffled how I could've missed that, I was specifically looking for similarities/differences. Thanks a lot! :) \$\endgroup\$ – Abbas Dec 19 '17 at 13:37
1
\$\begingroup\$

I wouldn't have a function that sets the event handler since this is usually something that is only done once. Instead of using $.keyup you can use assign the keyup listener to the document and have jQuery only respond when it lands on that element, this way you never need to call it more than once, which mean you definitely don't need to put it in a function.

$(document).on('keyup', "#myInput", function() {
    search(this.value, true);
});

function search(value, caseInsensitive){
    if(caseInsensitive) value = value.toLowerCase();
    $("table").find("tr").each(function(index) {
        if (index === 0) return;
        var id = $(this).find("td").first().text();
        if(caseInsensitive) id = id.toLowerCase();
        $(this).toggle(id.indexOf(value) !== -1);
    });
}

Your 2 search functions are similar enough that you can pass it a parameter to determine if it should be a case sensitive search or not. The searchFilt function seems redundant, but if you need it it would just be..

function searchFilt(value){ return search(value); }
\$\endgroup\$
  • \$\begingroup\$ +1 for assigning the event once, another thing, why don't you make your answer better by using contains:, along with overriding it using $.expr[":"].contains = $.expr.createPseudo(function(arg) {...... to use case-insensitive approach? it would eliminate .each, sorry if i am wrong here. \$\endgroup\$ – Muhammad Omer Aslam Dec 21 '17 at 0:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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