Auto-complete in JavaScript

Question

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.

function customAutoComplete(input,sourceList){

if(!input){
    console.error('No input box provided');
}    
if(!sourceList ||   sourceList.length<=0){
    console.error('No sourceList provided for customAutoComplete');
}

var suggList;
input.addEventListener('keyup',function(e){
    suggList=populateSourceList(input.value);
    if(suggList){ 
        input.parentNode.appendChild(suggList);
    }
    if(e.keyCode==40){
        suggList.focus();
    }
});

input.addEventListener('blur',function(e){
    if(e.relatedTarget.id!=input.id + '_autoComplete'){
        setTimeout(clearSuggestionList,200);             
    }
});



function populateSourceList(inputText){

    if(!inputText){
        return null;
    }       
    var suggList=getSuggestList();
    var size=1;
    sourceList.forEach( function(itm){
        if(itm.indexOf(inputText)>=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();
    } 
}
}   

Answer 1

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 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'

Answer 2

console.error('No input box provided');

input.addEventListener('keyup',function(e){

sourceList.forEach( function(itm){

If that company is a consulting firm whose clients involve enterprise, then expect weird browser requirements. That's because your clients may still be using IE6 for their intranet, or some old application with an embedded IE. That means the console, addEventListener, and forEach among others may not exist.

acList = document.createElement("select");

Next is your interesting approach of using <select> as your list. Most autocomplete lists usually build lists by creating an inline/inline-block <div> after the input, move the input to it, and then append an absolute <ul> to the <div>. The end result is something like:

.autocomplete{
  display: inline-block; 
  position: relative;
}

.autocomplete ul{
  background: #FFF;
  padding: 0;
  margin: 0;
  list-style: none;
  left: 0;
  right: 0;
  position: absolute;
  border: 1px solid #000;
  box-shadow: 0 5px 10px #999;
}

.autocomplete li{
  padding: 5px;  
  border-top: 1px solid #EEE;
}

.autocomplete ul li:first-child{
  border: none;  
}

.autocomplete li:hover{
  background: #EEE;  
  cursor: pointer;
}

<div class="autocomplete">
  <input type="text">
  <ul>
    <li>item 1</li>
    <li>item 2</li>
    <li>item 3</li>
  </ul>
</div>
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed facilisis varius arcu fringilla rutrum. Curabitur magna ipsum, maximus at fringilla vel, feugiat interdum quam. Duis ligula nisl, maximus eu iaculis nec, eleifend eget felis. Donec commodo pharetra ornare. Duis a pulvinar augue. Mauris rhoncus est vitae fringilla auctor. Maecenas venenatis ut mauris iaculis tincidunt. Proin est erat, hendrerit vel viverra at, venenatis sit amet mi. Proin efficitur, nisi eget egestas faucibus, augue diam egestas tellus, eu viverra risus ipsum ac nulla.</p>

The advantage of such is you can style the list as well as the items, something that you can't do or will have difficulty achieving with a <select>. The only thing that is lacking in the above code is to add and remove <li> from the list depending on your input. Notice that most of the styling is already in CSS, meaning JS will not have to worry about styles.

Another thing I notice is your function names and variable names. Companies have big code bases, with some methods a thousand lines long (yes, I've seen such horrors). That means there's a lot of variables in play in one block. If you name something as item - which item are you referring to? The function may have a lot of kinds of items in play. Name your variables and functions sensibly. They'll save you a lot of hair in the long run.

Usually, companies use robust backend frameworks like the ones in Java and C#. This means pages are pre-rendered and come with it's own tools and libraries. You should be careful in defining stuff. In this case, your function is in the global scope. You don't want to do that, it may clobber and/or get clobbered by some other framework. Wrap it in a strict-mode closure.

;(function(){
  'use strict';

  // Your stuff will run here, safe from the outside

}());

Some other things to note is the use of ==. That's loose comparison, and things like '' == false are true. Use strict comparison (===) whenever possible. Reserve loose comparison for special cases.