views:

58

answers:

2

Is there a better way to write this function? I've inherited some javascript code and I'd like to make this more concise if possible. Also, I'll probably be adding many more "theme" elements and don't want to copy and paste over and over.

function imageClick() {
var theme1 = document.getElementById("li-theme1");
var theme2 = document.getElementById("li-theme2");
var theme3 = document.getElementById("li-theme3");

var imgtheme1 = theme1.getElementsByTagName("img");
var imgtheme2 = theme2.getElementsByTagName("img");
var imgtheme3 = theme3.getElementsByTagName("img");

var inputtheme1 = document.getElementById("radiotheme1");
var inputtheme2 = document.getElementById("radiotheme2");
var inputtheme3 = document.getElementById("radiotheme3");

imgtheme1[0].onclick = function() {
    inputtheme1.checked = true;
    highlightChoice("li-theme1");
}
imgtheme2[0].onclick = function() {
    inputtheme2.checked = true;
    highlightChoice("li-theme2");
}
imgtheme3[0].onclick = function() {
    inputtheme3.checked = true;
    highlightChoice("li-theme3");
}
}
+2  A: 

I've "hardcoded" the imageClick() function to the ones that you've specified, but you could change this to be a "for(var i=1;i<4;i++) {imageClickItem(i);}" type loop if you wished.

function imageClick()
{
    imageClickItem(1);
    imageClickItem(2);
    imageClickItem(3);
}

function imageClickItem(itemNumber)
{
    var theme = document.getElementById("li-theme" + itemNumber);
    var imgtheme = theme.getElementsByTagName("img");
    var inputtheme = document.getElementById("radiotheme" + itemNumber);

    imgtheme[0].onclick = function()
    {
        inputtheme.checked = true;
        highlightChoice(theme.id);
    }
}
Rob
I also made the assumption that highlightChoice would always be called with the id of the element stored in "theme", based on your above code, if not you'd want to tweak the line third from bottom accordingly :)
Rob
+3  A: 
function imageClick() 
{
    for (var i=1; i<4; i++)
    { 
        var theme = document.getElementById("li-theme"+i);
        var imgtheme = theme.getElementsByTagName("img");
        imgtheme[0].onclick = (function (current) 
        { 
            return function() 
            {
                document.getElementById("inputtheme"+current) = true;
                highlightChoice("li-theme"+current);
            }
        })(i);
    }
} 

If you want to add more iterations at the later date, just increase the 4 in i<4 to the number of iterations you'd like to perform + 1.

Andy E