# Binding click event through elements

I need to know whether I am doing the best way of binding click event through elements in jQuery. I need to avoid duplicate code in my function (since lot of duplicate code is repeated in all functions).

How can I clean this up and do it in a better way?

 $(".nna-icon-excel").click(function (e) { var$this = $(this);$this.parent().attr("href", "?excel=1")
var href = $this.parent().attr("href"); if (activeTab.toLowerCase() == "table") { action = "expand"; } else { action = "expandheatmap"; }$this.parent().attr("href", href + "&from=" + action + "&expand=" + Expand + "");
});
$(".nna-icon-pdf").click(function (e) { var$this = $(this);$this.parent().attr("href", "?savehtml=1&openpdf=1&orientation=@ViewBag.PdfOrientation")
var href = $this.parent().attr("href"); if (activeTab.toLowerCase() == "table") { action = "expand"; } else { action = "expandheatmap"; }$this.parent().attr("href", href + "&from=" + action + "&tabactive=" + action + "&expand=" + Expand + "");
});
$(".gridoptions-anchor").click(function (e) { var$this = $(this);$this.attr("href", "/home/xx/Index" + "/?columnchooser=1");
var href = $(this).attr("href"); var action = ""; if (activeTab.toLowerCase() == "table") { action = "expand"; } else { action = "expandheatmap"; }$this.attr("href", href  + "&from=" +action + "&expand=" + Expand+ "");

});


You are right, the code could be tigher

• This

var action = "";
if  {
action = "expand";
}
else {
action = "expandheatmap";
}


could be reduced with a ternary operation to:

var action = (activeTab.toLowerCase() == 'table') ? 'expand' : 'expandheatmap;


Since you repeat this in every handle, you should consider building a function for this, in case the logic ever changes.

• I am a bit confused by your code, you seem to build the href, get the href, modify that string and then set the href again ? Why ? The following should work in my mind ( I took the last click handler ) :

$(".gridoptions-anchor").click(function (e) { var href = "/home/xx/Index" + "/?columnchooser=1" var action = (activeTab.toLowerCase() == 'table') ? 'expand' : 'expandheatmap;$(this).attr("href", href  + "&from=" +action + "&expand=" + Expand + "");
});

• This brings me to my next point; activeTab, action, and Expand are not defined with var in this code
• Finally you could consider a dedicated URL building function to have the string concatenation in one place:

function buildURL( href, action, expand, addTabActive )
{
var tabActiveParameter = addTabActive ? ( '&tabactive=' + action ) : '';
return href + "&from=" + action + "&expand=" + expand + tabActiveParameter;
}

• Which brings me to my last point if tabactive shows the action, then activetab would be a better name.
• whether am doing correct way of binding event to element? or need to use latest ".on" in jquery. i am bit confused about this binding technique. am i using standard way ? – SivaRajini Mar 7 '14 at 15:21
• Feel free to select this as the answer (green checkmark) – konijn Mar 7 '14 at 15:21
• .click() is fine, just dont use .live() – konijn Mar 7 '14 at 15:21
• Accepted!!! cheers!!! – SivaRajini Mar 7 '14 at 15:23