Avatar creator form

Question

I'm making an avatar creator with jQuery where users can select and change hair, eye and skin colour. The page is a form with different values and when the user clicks on the form options the values are translated to CSS. This allows the avatar options to change. jQuery enhanced cookie is used to store the data values.

Everything is working fine, except my code is very repetitive.

Here is an extract from my first file:

/* Capture Gender */

$(".gender input").click(function () {
    var gender = $(this).val();
    var input = 'input[value="hair' + gender + '1"]';
    $(input).attr('checked', 'checked');
    $("span#hair").removeAttr('class');
    $("span#hair").addClass('hair' + gender + '1');
    $.cookie(product + "hairstyle", 'hair' + gender + '1');
    var classes = $('#container').attr('class').split(' ');
    classes[1] = gender;
    $('#container').attr('class', classes.join(' '));
    $.cookie(product + "gender", gender);
});


/* Capture Hair Style */

$(".hairstyle input").click(function () {
    var newhairstyle = $(this).val();
    $("span#hair").removeAttr('class');
    $("span#hair").addClass(newhairstyle);
    $.cookie(product + "hairstyle", newhairstyle);
});


/* Capture Colour-Hair */

$(".haircolour input").click(function () {
    var newhaircolour = $(this).val();
    var classes = $('#container').attr('class').split(' ');
    classes[0] = newhaircolour;
    $('#container').attr('class', classes.join(' '));
    $.cookie(product + "haircolour", newhaircolour);
});

(This goes on for several lines (see here)).

I then have a second file, which repeats some of the code:

var hairstylestorage = $.cookie(product + "hairstyle"); 
if (hairstylestorage === null) {
    $.cookie(product + "hairstyle", defaulthairstyle);
} else {
        var newhairstyle = $.cookie(product + "hairstyle"); 
        $("span#hair").removeAttr('class');
        $("span#hair").addClass(newhairstyle);
        var input = 'input[value="'+ newhairstyle + '"]';
        $(input).attr('checked', 'checked');
}

var haircolourstorage = $.cookie(product + "haircolour"); 
if (haircolourstorage === null) {
    $.cookie(product + "haircolour", defaulthaircolour);
} else {
        var newhaircolour =  $.cookie(product + "haircolour"); 
        var classes = $('#container').attr('class').split(' ');
        classes[0] = newhaircolour;
        $('#container').attr('class', classes.join(' '));
}

This also goes on for several lines (see here).

How can I make it less repeatable? I thought I could use a function as shown on here and then pass through the variables that change. However, as a lot of my code uses functions, I'm not sure how to call a function inside of a function.

Answer 1

froderik is correct in that you should start by looking at the code and pick out similarities that can be extracted into functions. Doing this a little bit at a time will make it easier to read and then you will find more and more things to combine.

For instance, this chunk of code:

var classes = $('#container').attr('class').split(' ');
classes[0] = newhaircolour;
$('#container').attr('class', classes.join(' '));

becomes

function changeContainerClass(ix, value) {
    var classes = $('#container').attr('class').split(' ');
    classes[ix] = value;
    $('#container').attr('class', classes.join(' '));
}

And

$("#eyecolour").removeAttr('class');
$("#eyecolour").addClass(neweyecolour);

becomes

function setClass(propName, value) {
    $("#" + propName).removeAttr('class');
    $("#" + propName).addClass(value);
}

After those are replaced in various areas, your code now looks like this:

$(".eyecolour input").click(function () {
    var neweyecolour = $(this).val();
    setClass('eyecolour', neweyecolour);
    $.cookie(product + "eyecolour", neweyecolour);
});
$(".skincolour input").click(function () {
    var newskincolour = $(this).val();
    changeContainerClass(2, newskincolour)
    $.cookie(product + "skincolour", newskincolour);
});

Not a huge change but you can see a pattern forming.

1. Add click event to inputs in a div with a class name selector. (.eyecolour inputs)
2. Set the class of the div to the value of the clicked input.
3. Set the class of the container item.
4. Set the cookie.

Numbers 2 and 3 are not done every time but we will work around that.

So let's combine these items (and simplify the class setting):

function addClickHandler(propName, containerIndex) {
    $('.' + propName + ' input').click(function() {    
        var value = $(this).val();
        var input = $('#' + propName);
        if (input) { input.removeClass().addClass(value); } // if input exists, removes all classes and then adds new class
        if (containerIndex != null && containerIndex != undefined) { // if containerIndex is defined (can't use the truthy check like input because may be 0)
            changeContainerClass(containerIndex, value);
        }
        $.cookie(product + propName, value); // set cookie
    }
});

Now you can call this function like so:

addClickHandler('haircolour', 0);
addClickHandler('eyecolour'); // we leave off the containerIndex so it will be undefined and therfore ignored.

That is a pretty big savings right there. But we can use an array with these values and then just loop through each to add the handler.

 var inputs = [
                { prop: 'hairstyle' },
                { prop: 'haircolour', containerIndex: 0 },
                { prop: 'skincolour', containerIndex: 2 },
                { prop: 'eyecolour' },
                { prop: 'glasses' }
             ];

for (var i = 0; i < inputs.length; i++) {
    addClickHandler(inputs[i].prop, inputs[i].containerIndex);
}

Okay, now on to the other page. You have similar functionality but instead of pulling from the selected form item, you are pulling from the cookie values.

var newglasses = $.cookie(product + "glasses");
$("#glasses").removeAttr('class');
$("#glasses").addClass(newglasses);
$.cookie(product + "glasses", newglasses);    

and

var newhaircolour =  $.cookie(product + "haircolour"); 
var classes = $('#container').attr('class').split(' ');
classes[0] = newhaircolour;
$('#container').attr('class', classes.join(' '));

Well we have already refactored this stuff. And it is the body of the click event handler above. So lets change that to

function setValue(propName, value, containerIndex) {
     var input = $('#' + propName);
     if (input) { input.removeClass().addClass(value); } // if input exists, removes all classes and then adds new class
     if (containerIndex != null && containerIndex != undefined) { // if containerIndex is defined (can't use the truthy check like input because may be 0)
            changeContainerClass(containerIndex, value);
     }
     $.cookie(product + propName, value); // set cookie
}
function addClickHandler(propName, containerIndex) {
    $('.' + propName + ' input').click(function() {    
         setValue(propName, $(this).val(), containerIndex);  
    }
});

Now the cookie functionality becomes

var haircolourstorage = $.cookie(product + "haircolour"); 
if (haircolourstorage === null) {
    $.cookie(product + "haircolour", defaulthaircolour);
} else {
    var newhaircolour =  $.cookie(product + "haircolour"); 
    setValue("haircolour", newhaircolour);
}

And (I'm sure you saw this coming by now) that can be refactored into:

function loadFromCookie(prodName, containerIndex) {
    var storage = $.cookie(product + propName);
    if (storage === null) {
        var defaultval = $("." + propName + " input").val(); // this follows a standard pattern so we can just use that here.
        $.cookie(product + propName, defaultval);
    } else {
        var newValue = $.cookie(product + propName);
        this.setValue(propName, newValue, containerIndex);
    }
}

And that can be used using the same array as we setup before.

for (var i = 0; i < inputs.length; i++) {
    loadFromCookie(inputs[i].prop, inputs[i].containerIndex);
}

I have been using function declarations. function functionName() {} But there is also function expressions var functionName = function() {};. There are a few differences between these but for our purposes now they are the same (if you are interested, google javascript hoisting for one of the big differences). So I could have declared each of these expressions using that technique and used them exactly the same. In fact I can put these functions into another function and that will accomplish two things.

1. It will move all of these functions out of the global namespace (ALWAYS beneficial). This is not really a problem when you declare them inside jquery's document load function but still useful.

2. I can group these functions together into a "module" and stored in a separate file.

So that is what I did here:

var AvatarCreator = function (productName) {
    var product = productName;
    var inputs = [
                { prop: 'hairstyle' },
                { prop: 'haircolour', containerIndex: 0 },
                { prop: 'skincolour', containerIndex: 2 },
                { prop: 'eyecolour' },
                { prop: 'glasses' }
             ];  // we declare the array inside the function but it could just as easily be passed in like the product.

return {  // we return an object so that we can call functions later.

    // this function handles the changes to the container class, pass in the index and the value
    changeContainerClass: function (ix, value) {
        var classes = $('#container').attr('class').split(' ');
        classes[ix] = value;
        $('#container').attr('class', classes.join(' '));
    },
    // sets the value of a given property
    setValue: function (propName, value, containerIndex) {
        var input = $('#' + propName);
        if (input) { input.removeClass().addClass(value); } // if input exists, removes all classes and then adds new class
        if (containerIndex != null && containerIndex != undefined) { // if containerIndex is defined (can't use the truthy check like input because may be 0)
            this.changeContainerClass(containerIndex, value);
        }
        $.cookie(product + propName, value); // set cookie
    },
    addClickHandler: function (propName, containerIndex) { // this function adds the click event handler to the input.
        var that = this; // store the current object in temp function (otherwise we cannot access the setValue function)  This is a common practice.
        $('.' + propName + ' input').click(function () {
            that.setValue(propName, $(this).val(), containerIndex);
        });
    },
    loadFromCookie: function (propName, containerIndex) { // this handles the cookie retrieval (or storing of the default values)
        var storage = $.cookie(product + propName);
        if (storage === null) {
            //console.log(product + propName + ' not found.');
            var defaultval = $("." + propName + " input").val();
            $.cookie(product + propName, defaultval);
        } else {
            var newValue = $.cookie(product + propName);
            this.setValue(propName, newValue, containerIndex);
            //console.log(product + propName + 'found with value - ' + newValue);
        }
    },
    loadAllFromCookies: function () { 
        for (var i = 0; i < inputs.length; i++) {
            this.loadFromCookie(inputs[i].prop, inputs[i].containerIndex);
        }
    },
    addAllClickHandlers: function () {
        for (var i = 0; i < inputs.length; i++) {
            this.addClickHandler(inputs[i].prop, inputs[i].containerIndex);
        }
    }

};
}

That function can be stored in a separate file (AvatarCreator.js) and then included on the page as needed.

var product = $('body').attr("class");

var avatar = new AvatarCreator(product);
avatar.loadAllFromCookies();
// and/or
avatar.addAllClickHandlers();

I did not touch on the personal message, name, and a few others that you had in your code but did not have on the page. But with a few changes to the html to conform to the conventions we have now set up you can easily add those to the array and it will automatically take care of those as well.

The gender selector has a little extra functionality that needs to occur. You can leave that out of the array and handle it completely separately. You could pass that field into the constructor and handle is special within the module (this is probably what I would recommend). or you could even add a callback method to the array objects.

But I will leave that up to you. I think I have given you enough to think about.

Answer 2

You are perfectly right to be looking at functions as you way out of repetition. In this case the code:

$("span#hair").removeAttr('class');
$("span#hair").addClass('hair' + gender + '1');
$.cookie(product + "hairstyle", 'hair' + gender + '1');

appears one more time with a slightly different class name. You could put this in a function:

var changeHairstyle = function(hairClass){
    $("span#hair").removeAttr('class');
    $("span#hair").addClass(hairClass);
    $.cookie(product + "hairstyle", hairClass);
};

and call it (in the second case) with:

$(".hairstyle input").click(function () {
    var newhairstyle = $(this).val();
    changeHairstyle(newhairstyle);
});

This example illustrates well how to work with reuse. Look for similarities and put them in functions and you have a good start to writing code more efficiently. There are much more to learn but I think this is one of the most important lessons to learn.