开发者

How to improve this if else structure in JavaScript?

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));

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜