patternjavascriptMinor
Auto-complete in JavaScript
Viewed 0 times
autocompletejavascript
Problem
I recently got interviewed in a company for Senior level position. They asked me to develop auto-complete component (2 hours time limit) using plain JavaScript similar to this.
I wrote below code, which works as expected, but they responded saying that the solution doesn't match expectations. Please help me figure out what I could improve in this.
Here is the link to working example.
I wrote below code, which works as expected, but they responded saying that the solution doesn't match expectations. Please help me figure out what I could improve in this.
Here is the link to working example.
function customAutoComplete(input,sourceList){
if(!input){
console.error('No input box provided');
}
if(!sourceList || sourceList.length=0) {
var listItem=document.createElement("option");
listItem.appendChild(document.createTextNode(itm));
suggList.appendChild(listItem);
size++;
}
});
suggList.setAttribute('size',size);
if(size>1){
return suggList;
}
return null;
}
function getSuggestList(){
var acListId=input.id + '_autoComplete';
var acList= document.getElementById(acListId)
if(!acList){
acList = document.createElement("select");
acList.setAttribute('class','suggestion-list');
acList.setAttribute('id',acListId);
var stl='left:' + input.offsetLeft + 'px;top:' + (input.offsetTop + input.offsetHeight) + 'px;min-width:' + input.offsetWidth + 'px';
acList.setAttribute('style',stl);
acList.addEventListener('keydown',function(e){autoCompleteItemSelected(e,acList)});
acList.addEventListener('blur', function(){clearSuggestionList(suggList)} );
}
else{
acList.innerHTML='';
}
return acList;
}
function clearSuggestionList(suggList){
if(!suggList){
suggList=getSuggestList();
}
suggList.outerHTML='';
}
function autoCompleteItemSelected(e,list){
if(e.keyCode==13){
input.value=list.selectedOptions[0].text;
clearSuggestionList();
}
}
}Solution
There's a few things that can be reviewed:
Your indentation and spacing are incorrect, use JSFiddle's TidyUp function to clean it up.
All of the
Don't sacrifice readability for a few characters:
Instead of
Your indentation and spacing are incorrect, use JSFiddle's TidyUp function to clean it up.
All of the
e.keyCode == matches shouldn't be matched to magic numbers, but moved to a kind of enum or something similar:var KEYS = {
Enter: 13,
DownArrow: 40,
UpArrow: 38
};
// ...
if (e.keyCode == KEYS.Enter){suggList:Don't sacrifice readability for a few characters:
suggestedList is more than fine.console.error:Instead of
console.error, use throw instead: console.error('No sourceList provided for customAutoComplete') into: throw 'ACError: No sourceList provided for customAutoComplete'Code Snippets
var KEYS = {
Enter: 13,
DownArrow: 40,
UpArrow: 38
};
// ...
if (e.keyCode == KEYS.Enter){Context
StackExchange Code Review Q#108214, answer score: 3
Revisions (0)
No revisions yet.