Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

DOM querying is expensive

###DOM querying is expensive SearchSearch on Google for "js DOM query expensive" and you will likely find many posts from the past 10 years that discuss how in-efficient it is to be querying the DOM each time. Stop Writing Slow Javascript appears to be somewhat recent - see the section Cache DOM Lookups. This answer on SO about various DOM-selector functions will likely be interesting as well.

Keep the scopes limited

###Keep the scopes limited AnotherAnother thing that article mentions is limiting the scope as much as possible (see the section Keep your scopes close and your scope even closer). One way to not clutter up the global namespace is to wrap the code with an IIFE:

###DOM querying is expensive Search on Google for "js DOM query expensive" and you will likely find many posts from the past 10 years that discuss how in-efficient it is to be querying the DOM each time. Stop Writing Slow Javascript appears to be somewhat recent - see the section Cache DOM Lookups. This answer on SO about various DOM-selector functions will likely be interesting as well.

###Keep the scopes limited Another thing that article mentions is limiting the scope as much as possible (see the section Keep your scopes close and your scope even closer). One way to not clutter up the global namespace is to wrap the code with an IIFE:

DOM querying is expensive

Search on Google for "js DOM query expensive" and you will likely find many posts from the past 10 years that discuss how in-efficient it is to be querying the DOM each time. Stop Writing Slow Javascript appears to be somewhat recent - see the section Cache DOM Lookups. This answer on SO about various DOM-selector functions will likely be interesting as well.

Keep the scopes limited

Another thing that article mentions is limiting the scope as much as possible (see the section Keep your scopes close and your scope even closer). One way to not clutter up the global namespace is to wrap the code with an IIFE:

move function call, mention event delegation
Source Link
document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
 addEvent(ul);
});

You will also notice that in that event callback, the unordered list (i.e. ul) is added to the document and hidden. That way there is only one list ever added, and because of that, we can hide and show the list instead of removing it. The function removeChild can be replaced with a function hideList that will set the display style (using HTMLElement.style) to none. Additionally, the call to addEvent() was moved into that callback function, so we only add the event listener once. One could also use event delegation and have one click handler for the whole page - it would just handle clicks differently depending on the type of element clicked.

'use strict';;
;(function(window, document, undefined) {
 var data = ['alabama', 'alaska', 'arizona', 'arkansas', 'california', 'colorado', 'connecticut', 'delaware'];
 /** 
 * variables: searchBox is where you type the city name, ul is the list of suggestions
 */
 var searchBox, ul;
 /**
 * methods
 */
 function getElementById(id) {
 return document.getElementById(id);
 }
 function iterate(arr, callback) {
 for (var i = 0; i < arr.length; i++) {
 callback(arr[i], i);
 }
 }
 var search = function(val) {
 var arr = [];
 iterate(data, function(item, index) {
 if (item.indexOf(val) !== -1) {
 arr.push(item);
 }
 })
 return arr;
 }
 var addEvent = function(elm) {
 elm.addEventListener('click', function(event) {
 console.log('list click - target: ',event.target,' currentTarget: ',event.currentTarget);
 if (event.target !== event.currentTarget) {
 searchBox.value = event.target.textContent;
 hideList();
 }
 event.stopPropagation();
 });
 }
 var appendChild = function(parent, child) {
 parent.appendChild(child);
 }
 var hideList = function() {
 ul.style.display = 'none';
 while (ul.firstChild) {
 ul.removeChild(ul.firstChild);
 }
 }
 var bindData = function(arr) {
 ul.style.display = '';
 iterate(arr, function(item, index) {
 var li = document.createElement('li');
 var a = document.createElement('a');
 a.textContent = item;
 appendChild(li, a)
 appendChild(ul, li);
 });
 addEvent(ul);
 }
 function debounce(func, wait, immediate) {
 var timeout;
 return function() {
 var context = this,
 args = arguments;
 var later = function() {
 timeout = null;
 if (!immediate) {
 func.apply(context, args);
 }
 };
 var callNow = immediate && !timeout;
 clearTimeout(timeout);
 timeout = setTimeout(later, wait);
 if (callNow) {
 func.apply(context, args);
 }
 };
 }
 var typeAhead = debounce(function(event) {
 hideList(); //removeChild();
 var value = event.target.value;
 if (value !== '') {
 var arr = search(value);
 bindData(arr);
 }
 }, 400);
 document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
 addEvent(ul);
 });
})(window, document);
<input type="text" placeholder="type to search" id="typeahead">
document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
});

You will also notice that in that event callback, the unordered list (i.e. ul) is added to the document and hidden. That way there is only one list ever added, and because of that, we can hide and show the list instead of removing it. The function removeChild can be replaced with a function hideList that will set the display style (using HTMLElement.style) to none.

'use strict';;
;(function(window, document, undefined) {
 var data = ['alabama', 'alaska', 'arizona', 'arkansas', 'california', 'colorado', 'connecticut', 'delaware'];
 /** 
 * variables: searchBox is where you type the city name, ul is the list of suggestions
 */
 var searchBox, ul;
 /**
 * methods
 */
 function getElementById(id) {
 return document.getElementById(id);
 }
 function iterate(arr, callback) {
 for (var i = 0; i < arr.length; i++) {
 callback(arr[i], i);
 }
 }
 var search = function(val) {
 var arr = [];
 iterate(data, function(item, index) {
 if (item.indexOf(val) !== -1) {
 arr.push(item);
 }
 })
 return arr;
 }
 var addEvent = function(elm) {
 elm.addEventListener('click', function(event) {
 if (event.target !== event.currentTarget) {
 searchBox.value = event.target.textContent;
 hideList();
 }
 event.stopPropagation();
 });
 }
 var appendChild = function(parent, child) {
 parent.appendChild(child);
 }
 var hideList = function() {
 ul.style.display = 'none';
 while (ul.firstChild) {
 ul.removeChild(ul.firstChild);
 }
 }
 var bindData = function(arr) {
 ul.style.display = '';
 iterate(arr, function(item, index) {
 var li = document.createElement('li');
 var a = document.createElement('a');
 a.textContent = item;
 appendChild(li, a)
 appendChild(ul, li);
 });
 addEvent(ul);
 }
 function debounce(func, wait, immediate) {
 var timeout;
 return function() {
 var context = this,
 args = arguments;
 var later = function() {
 timeout = null;
 if (!immediate) {
 func.apply(context, args);
 }
 };
 var callNow = immediate && !timeout;
 clearTimeout(timeout);
 timeout = setTimeout(later, wait);
 if (callNow) {
 func.apply(context, args);
 }
 };
 }
 var typeAhead = debounce(function(event) {
 hideList(); //removeChild();
 var value = event.target.value;
 if (value !== '') {
 var arr = search(value);
 bindData(arr);
 }
 }, 400);
 document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
 });
})(window, document);
<input type="text" placeholder="type to search" id="typeahead">
document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
 addEvent(ul);
});

You will also notice that in that event callback, the unordered list (i.e. ul) is added to the document and hidden. That way there is only one list ever added, and because of that, we can hide and show the list instead of removing it. The function removeChild can be replaced with a function hideList that will set the display style (using HTMLElement.style) to none. Additionally, the call to addEvent() was moved into that callback function, so we only add the event listener once. One could also use event delegation and have one click handler for the whole page - it would just handle clicks differently depending on the type of element clicked.

'use strict';;
;(function(window, document, undefined) {
 var data = ['alabama', 'alaska', 'arizona', 'arkansas', 'california', 'colorado', 'connecticut', 'delaware'];
 /** 
 * variables: searchBox is where you type the city name, ul is the list of suggestions
 */
 var searchBox, ul;
 /**
 * methods
 */
 function getElementById(id) {
 return document.getElementById(id);
 }
 function iterate(arr, callback) {
 for (var i = 0; i < arr.length; i++) {
 callback(arr[i], i);
 }
 }
 var search = function(val) {
 var arr = [];
 iterate(data, function(item, index) {
 if (item.indexOf(val) !== -1) {
 arr.push(item);
 }
 })
 return arr;
 }
 var addEvent = function(elm) {
 elm.addEventListener('click', function(event) {
 console.log('list click - target: ',event.target,' currentTarget: ',event.currentTarget);
 if (event.target !== event.currentTarget) {
 searchBox.value = event.target.textContent;
 hideList();
 }
 event.stopPropagation();
 });
 }
 var appendChild = function(parent, child) {
 parent.appendChild(child);
 }
 var hideList = function() {
 ul.style.display = 'none';
 while (ul.firstChild) {
 ul.removeChild(ul.firstChild);
 }
 }
 var bindData = function(arr) {
 ul.style.display = '';
 iterate(arr, function(item, index) {
 var li = document.createElement('li');
 var a = document.createElement('a');
 a.textContent = item;
 appendChild(li, a)
 appendChild(ul, li);
 });
 }
 function debounce(func, wait, immediate) {
 var timeout;
 return function() {
 var context = this,
 args = arguments;
 var later = function() {
 timeout = null;
 if (!immediate) {
 func.apply(context, args);
 }
 };
 var callNow = immediate && !timeout;
 clearTimeout(timeout);
 timeout = setTimeout(later, wait);
 if (callNow) {
 func.apply(context, args);
 }
 };
 }
 var typeAhead = debounce(function(event) {
 hideList(); //removeChild();
 var value = event.target.value;
 if (value !== '') {
 var arr = search(value);
 bindData(arr);
 }
 }, 400);
 document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
 addEvent(ul);
 });
})(window, document);
<input type="text" placeholder="type to search" id="typeahead">
Source Link

###DOM querying is expensive Search on Google for "js DOM query expensive" and you will likely find many posts from the past 10 years that discuss how in-efficient it is to be querying the DOM each time. Stop Writing Slow Javascript appears to be somewhat recent - see the section Cache DOM Lookups. This answer on SO about various DOM-selector functions will likely be interesting as well.

In the code below, notice that the variables searchBox and ul are declared at the top

var searchBox, ul;

Those variables don't get assigned until the callback for a new event listener for the DOMContentLoaded event is triggered.

document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
});

You will also notice that in that event callback, the unordered list (i.e. ul) is added to the document and hidden. That way there is only one list ever added, and because of that, we can hide and show the list instead of removing it. The function removeChild can be replaced with a function hideList that will set the display style (using HTMLElement.style) to none.

###Keep the scopes limited Another thing that article mentions is limiting the scope as much as possible (see the section Keep your scopes close and your scope even closer). One way to not clutter up the global namespace is to wrap the code with an IIFE:

;(function(window, document, undefined) {
 var data = ['alabama', 'alaska', 'arizona', 'arkansas', 'california', 'colorado', 'connecticut', 'delaware'];
 //...
})(window, document);

See the changes applied below. There are likely other improvements that can be made as well...

'use strict';;
;(function(window, document, undefined) {
 var data = ['alabama', 'alaska', 'arizona', 'arkansas', 'california', 'colorado', 'connecticut', 'delaware'];
 /** 
 * variables: searchBox is where you type the city name, ul is the list of suggestions
 */
 var searchBox, ul;
 /**
 * methods
 */
 function getElementById(id) {
 return document.getElementById(id);
 }
 function iterate(arr, callback) {
 for (var i = 0; i < arr.length; i++) {
 callback(arr[i], i);
 }
 }
 var search = function(val) {
 var arr = [];
 iterate(data, function(item, index) {
 if (item.indexOf(val) !== -1) {
 arr.push(item);
 }
 })
 return arr;
 }
 var addEvent = function(elm) {
 elm.addEventListener('click', function(event) {
 if (event.target !== event.currentTarget) {
 searchBox.value = event.target.textContent;
 hideList();
 }
 event.stopPropagation();
 });
 }
 var appendChild = function(parent, child) {
 parent.appendChild(child);
 }
 var hideList = function() {
 ul.style.display = 'none';
 while (ul.firstChild) {
 ul.removeChild(ul.firstChild);
 }
 }
 var bindData = function(arr) {
 ul.style.display = '';
 iterate(arr, function(item, index) {
 var li = document.createElement('li');
 var a = document.createElement('a');
 a.textContent = item;
 appendChild(li, a)
 appendChild(ul, li);
 });
 addEvent(ul);
 }
 function debounce(func, wait, immediate) {
 var timeout;
 return function() {
 var context = this,
 args = arguments;
 var later = function() {
 timeout = null;
 if (!immediate) {
 func.apply(context, args);
 }
 };
 var callNow = immediate && !timeout;
 clearTimeout(timeout);
 timeout = setTimeout(later, wait);
 if (callNow) {
 func.apply(context, args);
 }
 };
 }
 var typeAhead = debounce(function(event) {
 hideList(); //removeChild();
 var value = event.target.value;
 if (value !== '') {
 var arr = search(value);
 bindData(arr);
 }
 }, 400);
 document.addEventListener("DOMContentLoaded", function(event) {
 searchBox = getElementById('typeahead');
 searchBox.addEventListener('keyup', typeAhead);
 ul = document.createElement('ul');
 ul.setAttribute('id', 'searchResults')
 appendChild(document.body, ul);
 hideList();
 });
})(window, document);
<input type="text" placeholder="type to search" id="typeahead">

default

AltStyle によって変換されたページ (->オリジナル) /