How to improve the following if-else structure in JavaScript?
if(isIE())
if(termin != "")
TargetElement.onclick = function() {merkzettelRemove(this, id, termin)};
else
TargetElement.onclick = function() {merkzettelRemove(this, id)};
else
if(termin != "")
TargetElement.setAttribute("onclick","merkzet开发者_运维知识库telRemove(this, " + id + ",
'" + termin + "')");
else
TargetElement.setAttribute("onclick","merkzettelRemove(this, " + id + ")");
// get a cross-browser function for adding events
var on = (function(){
if (window.addEventListener) {
return function(target, type, listener){
target.addEventListener(type, listener, false);
};
}
else {
return function(object, sEvent, fpNotify){
object.attachEvent("on" + sEvent, fpNotify);
};
}
}());
// add the event listener
on(TargetElement, "click", function(){
// if termin is empty we pass undefined, this is the same as not passing it at all
merkzettelRemove(this, id, (termin) ? termin : undefined);
});
First things first, put some more braces in. It'll make it clearer what's going on, as well as saving untold heartache when you come to edit this in future.
Yes, you can get away without the braces if they wrap a single statement. When you come along three months from now and add something else into one of those blocks, your code will break unless you remember to wrap the whole block in braces. Get into the habit of doing it from the beginning.
First, use { } everywhere, it will make your loops more readable.
Second: I think this does the same
if(isIE()) {
TargetElement.onclick = function() {
merkzettelRemove(this, id, termin || null);
};
} else {
TargetElement.setAttribute("onclick",
"merkzettelRemove(this, " + id + ",'" + termin || null + "')");
}
Third, but you could try using unobtrusive javascript to add handlers to TargetElement
Firstly, lose the browser-sniffing. onclick= function...
works everywhere; setAttribute
is not only broken in IE, but also ugly. JavaScript-in-a-string is a big code smell; avoid. setAttribute
on HTML attributes has IE problems and is less readable than simple DOM Level 1 HTML properties anyway; avoid.
Secondly, it would be ideal to make merkzettelRemove
accept an out-of-band value (null
, undefined
, or even ''
itself) as well as an omitted argument. It is possible it already allows undefined
, depending on what mechanism it is using to support optional arguments. If so you could say simply:
TargetElement.onclick= function() {
merkzettelRemove(this, id, termin || undefined);
};
If you must completely omit the argument and you don't want to use an if...else
, there's another way around although IMO the clarity is worse:
TargetElement.onclick= function() {
merkzettelRemove.apply(null, termin==''? [this, id] : [this, id, termin]);
};
I think using a Javascript framework is the best solution, but you can try something like this:
var src="merkzettelRemove(this, " + id + (termin && (",'" + termin + "'")) + ")";
if (isIE()) {
TargetElement.onclick = new Function(src);
} else {
TargetElement.setAttribute("onclick", src);
}
I know this is not an answer for your question, but if you don't have any reason for not doing so you should definitely use a library that cares about compatibility issues for you, such as jQuery. Then you could write something like this:
var el = $(TargetElement);
if (termin != "")
el.click(function() {merkzettelRemove(this, id, termin)});
else
el.click(function() {merkzettelRemove(this, id)});
how about using short circuiting operators. So the following code
if(A) {
if(B) {
C; }
else{
D;}
else{
if(E)
F;
else
G;
}
}
Will become
A && ((B&&C) || D || ((E&&F) || G));
精彩评论