Problem
This is my code:
function search() {
$("#myInput").keyup(function () {
var value = this.value;
$("table").find("tr").each(function (index) {
if (index === 0) return;
var id = $(this).find("td").first().text();
$(this).toggle(id.indexOf(value) !== -1);
});
});
}
$('body').on('click', '#searchFonts', function () {
var value = "Font"
$("table").find("tr").each(function (index) {
if (index === 0) return;
var id = $(this).find("td").first().text();
$(this).toggle(id.indexOf(value) !== -1);
});
});
$('body').on('click', '#searchScripts ', function () {
var value = "script"
$("table").find("tr").each(function (index) {
if (index === 0) return;
var id = $(this).find("td").first().text();
$(this).toggle(id.indexOf(value) !== -1);
});
});
I’m looking for ways to make this run more effectively, as I’m sure there is a more efficient way to search for the specific value’s seen below such as ‘Font’ and ‘Script’.
Solution
D.R.Y.
There is a widely accepted principle amongst developers: Don’t Repeat Yourself. The following block appears three times (with varying spacing):
$("table").find("tr").each(function(index) {
if (index === 0) return;
var id = $(this).find("td").first().text();
$(this).toggle(id.indexOf(value) !== -1);
});
That can be abstracted into a function (which could accept a parameter for the value
) which could then be called in place of that repeated block.
For example:
function toggleRowsWithKeyword(value) {
$("table").find("tr").each(function(index) {
if (index === 0) return;
var id = $(this).find("td").first().text();
$(this).toggle(id.indexOf(value) !== -1);
});
}
And usage:
function search() {
$("#myInput").keyup(function() {
toggleRowsWithKeyword(this.value);
});
}
$('body').on('click','#searchFonts',function(){
toggleRowsWithKeyword("Font");
});
$('body').on('click','#searchScripts ',function(){
toggleRowsWithKeyword("script");
});
Cache DOM lookups
Even with abstracting that code into a function, it would still be performing a query on the DOM.
$("table").find("tr")
That can be stored in a variable (or constant if ES-6’s const is used):
var rows = $("table").find("tr");
To be on the safe side, that assignment can be placed inside a DOM-loaded callback (e.g. a function wrapped to the jQuery callback wrapper: $()
):
$(function() { //DOM is ready
var rows = $("table").find("tr");
//Define toggleRowsWithKeyword(), add click handlers, etc.
});
Then the function can utilize rows
instead of querying for the rows:
rows.each(function(index) {...
CSS Selector for all except first row
The CSS Selector:
var rows = $("table").find("tr")
Could be simplified to:
var rows = $("tr")
And the check for the first row (i.e. if (index === 0) return;
) could be eliminated using the :not pseudo-class along with :first-child:
var rows = $("tr:not(:first-child)")
More tips in article
For more tips about improving Javascript that interacts with the DOM, see this post: Stop Writing Slow Javascript. I know it bashes jQuery initially but it has some very helpful tips (and nice quotes).