开发者

Reducing JavaScript weight for repetitive function

I'm not exactly a JS pro, and while not pretty or efficient, this works.

Effectively I have repeating field groups in the form, and allow the user to copy data from the first to all 5 if they want to.

How do I make this code more efficient?

function data_copy() {
if(document.pixelform.copy[0].checked){

document.pixelform.pixelpostclickinterval2.value=document.pixelform.pixelpostclickinterval1.value;
document.pixelform.pixelpostviewinterval2.value=document.pixelform.pixelpostviewinterval1.value;
document.pixelform.pixelsegmentinterval2.value=document.pixelform.pixelsegmentinterval1.value;

document.pixelform.pixelpostclickinterval3.value=document.pixelform.pixelpostclickinterval1.value;
document.pixelform.pixelpostviewinterval3.value=document.pixelform.pixelpostviewinterval1.value;
document.pixelform.pixelsegmentinterval3.value=document.pixelform.pixelsegmentinterval1.value;

document.pixelform.pixelpostclickinterval4.value=document.pixelform.pixelpostclickinterval1.value;
document.pixelform.pixelp开发者_JS百科ostviewinterval4.value=document.pixelform.pixelpostviewinterval1.value;
document.pixelform.pixelsegmentinterval4.value=document.pixelform.pixelsegmentinterval1.value;

document.pixelform.pixelpostclickinterval5.value=document.pixelform.pixelpostclickinterval1.value;
document.pixelform.pixelpostviewinterval5.value=document.pixelform.pixelpostviewinterval1.value;
document.pixelform.pixelsegmentinterval5.value=document.pixelform.pixelsegmentinterval1.value;

} else {

document.pixelform.pixelpostclickinterval2.value="";
document.pixelform.pixelpostviewinterval2.value="";
document.pixelform.pixelsegmentinterval2.value="";

document.pixelform.pixelpostclickinterval3.value="";
document.pixelform.pixelpostviewinterval3.value="";
document.pixelform.pixelsegmentinterval3.value="";

document.pixelform.pixelpostclickinterval4.value="";
document.pixelform.pixelpostviewinterval4.value="";
document.pixelform.pixelsegmentinterval4.value="";

document.pixelform.pixelpostclickinterval5.value="";
document.pixelform.pixelpostviewinterval5.value="";
document.pixelform.pixelsegmentinterval5.value="";

}
}


Use a loop:

for (var i = 2; i <= 5; i++)
{
  document.pixelform["pixelpostclickinterval" + i].value=document.pixelform.pixelpostclickinterval1.value;
  document.pixelform["pixelpostviewinterval" + i].value=document.pixelform.pixelpostviewinterval1.value;
  document.pixelform["pixelsegmentinterval" + i].value=document.pixelform.pixelsegmentinterval1.value;
}


What you have is most efficient in terms of performance, but for maintainability, this is preferred. JavaScript objects are dictionaries and as such, properties may be accessed using index key names.

var postclickinterval = "";
var postviewinterval = "";
var segmentinterval = "";

if (document.pixelform.copy[0].checked)
{
    postclickinterval = document.pixelform.pixelpostclickinterval1.value;
    postviewinterval = document.pixelform.pixelpostviewinterval1.value;
    segmentinterval = document.pixelform.pixelsegmentinterval1.value;
}


for (var i = 2; i <= 5; i++)
{
    document.pixelform["pixelpostclickinterval" + i].value = postclickinterval;
    document.pixelform["pixelpostviewinterval" + i].value = postviewinterval;
    document.pixelform["pixelsegmentinterval" + i].value = segmentinterval;
}


You can use loop and array.

 var pixelInfo = ['postview','postscript','segment'];
 function copydata() {
       if (document.pixelform.copy[0].checked){
           for (var i=2; i<6; i++) {
                for (var j=0; j<3; j++) {
                (function(idx,info){
                     document.pixelform['pixel'+pixelInfo[info]+idx].value =
                         document.pixelform['pixel'+pixelInfo[info]+'1'].value;
                 })(i,j);
              }
            }
         } else {
            for (var i=2; i<6; i++) {
                for (var j=0; j<3; j++) {
                    (function(idx,info){
                         document.pixelform['pixel'+pixelInfo[info]+idx].value = "";
                 })(i,j);
              }
            }
         }
      }


I would recommend to cache a reference to document.pixelform to avoid querying the DOM for the same elements over and over again, and to avoid excessive property lookups.

// Look up the elements only once
var pf = document.pixelform,
    pci2 = pf.pixelpostclickinterval2,
    pci3 = pf.pixelpostclickinterval3,
    pci4 = pf.pixelpostclickinterval4,
    pci5 = pf.pixelpostclickinterval5,
    pvi2 = pf.pixelpostviewinterval2,
    pvi2 = pf.pixelpostviewinterval2,
    pvi2 = pf.pixelpostviewinterval2,
    pvi2 = pf.pixelpostviewinterval2,
    pvi2 = pf.pixelpostviewinterval2,
    si2 = pf.pixelsegmentinterval2,
    si3 = pf.pixelsegmentinterval3,
    si4 = pf.pixelsegmentinterval4,
    si5 = pf.pixelsegmentinterval5;

function data_copy() {

    var checked = pf.copy[0].checked,
        pci = pci1.value,
        pvi = pvi1.value,
        si = si1.value;

    pci2.value = checked ? pci : '';
    pvi2.value = checked ? pvi : '';
    si2.value = checked ? si : '';

    pci3.value = checked ? pci : '';
    pvi3.value = checked ? pvi : '';
    si3.value = checked ? si : '';

    pci4.value = checked ? pci : '';
    pvi4.value = checked ? pvi : '';
    si4.value = checked ? si : '';

    pci5.value = checked ? pci : '';
    pvi5.value = checked ? pvi : '';
    si5.value = checked ? si : '';

}
0

上一篇:

下一篇:

精彩评论

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

最新问答

问答排行榜