Tokenizer object

Question

I was interested in how this code can be improved apart from basic caching. This object creates 'tokens' much like that the iOS mail app and the tag section below.

var TokenTime = {
assets : {
    tokenArray : []
},

init : function (){
    var self = this;
    //add event handler
    $('#friend').on('keyup', function (event) {
        //test data to make sure its usable
        self.testData( $(this) , event  );

    });
},

testData : function ( obj , event ) {
    var self = this;

    if ( event.keyCode == 32 || event.keyCode == 13 ) {
        //create a local variable and store value
        var token = obj.val().split(' ');

        //make the array
        self.arrayMaker( obj, token );
    } 

},

arrayMaker : function ( obj, token ) {
    var self = this;
    if( token[0] != "" ) {

        self.assets.tokenArray.push(token[0]);
        var html = $('<li>' + self.assets.tokenArray[self.assets.tokenArray.length - 1] + '<span data-num=' + (self.assets.tokenArray.length - 1)  + '>x</span></li>');

        obj.before(html);
        obj.val("");

     html.on('click', function (e) {
        $(this).remove();
        var test = $(this).find('span').data('num');

     });

    } else {

        obj.val('');

    }
}
}

TokenTime.init();

Answer 1

First of all, there are jQuery plugins out there, that do this for you. Tokeninput, for example, comes to mind. But if you want to roll your own, by all means do so. I'll continue under that assumption.

So here, in no particular order, are my observations

1. Now, you've hardcoded the #friend selector. A better approach would be passing it as an argument to init(). Makes the code generic and reusable.

2. If you do so, you'll also have to find another solution than the assets.tokenArray array, since that won't work if you're using your code for more than 1 input. (There are other problems, too. More on this later)
   Conversely, if you really do only need this for exactly 1 input, there are simpler structures you could use. For instance, just wrapping everything in a function.

3. You're also adding li elements to... well, I don't know exactly, but it's not valid HTML. You're inserting the LIs immediately before the input element, which means that either the LIs are not in an OL or UL element (which is invalid), or that the input is in an OL or UL element (which is also invalid). The elements should show up anyway, but you can't be sure how different browsers will react, since one way or another it's not valid markup. I'd suggest passing a 2nd selector/element to init() to specify the element that the LIs should be appended to.

4. You're inserting the token text directly into the HTML string you use to build the LI-elements. This can cause strange behaviour if someone types some HTML in the input field. Probably better to create an empty LI and then add the text using .text().

5. You have a SPAN element in the LIs, which I would imagine is there to remove the token. However, you're attaching the click-handler to the entire LI. So click anywhere on the token-LI, and the token is deleted. Not sure that's what you're going for.

6. Speaking of, your code seems incomplete when it comes to the removing the token. You have the num data-property on the span, which I imagine is there so you can find the right index in the tokenArray and remove it. However, this won't work. For instance, if you add 3 tokens, they'll have num values 0, 1, and 2. So far, so good. If you remove the 2nd token, you'll have 2 tokens left with num values 0 and 2. However, tokenArray only has 2 elements now, numbered 0 and 1, so now you can't properly remove the last token anymore, because its num (2) doesn't correspond to a valid index in the array. And if you add a new token its num will be 2, so you now have 3 LI elements with nums 0, 2, and 2, and the linking between the array and the elements is messed up.
   The simplest solution is simply to use the LIs themselves as the array. Add their "raw" token text as a data property on the LI, and when you need an array of tokens, loop through the LIs, picking out that property (using jQuery's .map(), for example).

7. You're using split() to get rid of spaces, but jQuery has a $.trim() function ("trim" being the usual name for a function that removes leading and trailing whitespace) that you can use instead. Of course, you could also just listen for a keydown.

8. You keep declaring a self variable, even when not necessary. Besides, jQuery has a $.proxy() function that you can use to bind your event handler to a certain context.

9. Your naming is a bit off. arrayMaker doesn't make an array, testData doesn't so much test data as it handles an event (and it doesn't actually test/check the token at all). I'd suggest addToken and handleKeyup as more descriptive names.

Here's a demo of how I'd do it. It works for multiple inputs, too. Of course, it should really be a jQuery plugin, but I'll leave that as an exercise to the reader.

Here's the code

// Set up event handlers etc.
function tokenize(input, listElement) {
    // wrap the elements/selectors
    input = $(input);
    listElement = $(listElement);

    // event handler to remove a token to the list element
    function removeToken() {
        $(this).parent().remove();
    }

    // internal function to add tokens to the list element
    function addToken(text) {
        var item = $("<li></li>"),
            removeButton = $("<span>X</span>");

        removeButton.on("click", removeToken);

        item.text(text).data("token", text).append(removeButton);
        listElement.append(item);
    }

    // attach the keyup event handler
    input.on("keyup", function(event) {
        var token;
        if (event.keyCode == 32 || event.keyCode == 13) {
            token = $.trim(input.val());
            if (token) {
                input.val("");
                addToken(token);
            }
        }
    });
}

// Get an array of tokens from a list element
function tokens(list) {
    return $(list).children("li").map(function() {
        return $(this).data("token");
    }).toArray();
}

Answer 2

You should also wrap your code into a namespace with IIFE to avoid global conflict.

i.e.

(function (token, undefined) {
    token.init = function () {
        var self = this;
        //add event handler
        $('#friend').on('keyup', function (event) {
            //test data to make sure its usable
            self.testData( $(this) , event  );

        });
    };
}(window.token = window.token || {}));