v0.4 Release – Optimizing Client Side Code

Posted on January 17, 2009 by Sid

This is concerning bug 459727. As one of my goals for this release I’m trying to maintain/add functionality but reduce the amount of code. I started off by perusing the code in renderMorePushLogResults() found in the hg_templates/pushlog.tmpl file (see below).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115

function renderMorePushLogResults() {
  var end = start;
  start = start - 20
  var pushCheckins = new XMLHttpRequest();
  pushCheckins.open('GET', '/json-pushes?startID=' + start + '&endID=' + end, true);
  pushCheckins.onreadystatechange = function() {
    if(pushCheckins.readyState == 4)  {
      if(pushCheckins.status != 404) {
        var loader = document.getElementById("loader");
        loader.innerHTML = '<img src="{url}static/ajax-loader.gif" align="right" />';
        var pushData = new Function("return " + pushCheckins.responseText) ();
        var counter = 0;
        for(var i = end; i > start; i--) {
          var trScroll = document.createElement("tr");
          if(counter == 0) {
            trScroll.className = "parity0"; 
            counter = 1;
          } else {
              counter = 0;
              trScroll.className = "parity1";
          }      
          var tdScrollUser = document.createElement("td");
          tdScrollUser.width = "184px";
          tdScrollUser.innerHTML += pushData[i].user + '<br />' + pushData[i].formattedDate;
          //Create changset link
          var tdScrollChangeset = document.createElement("td");
          tdScrollChangeset.innerHTML += 
            '<a href="/rev/' + 
            pushData[i].individualChangeset.substring(0, 12) + 
            '">' + 
            pushData[i].individualChangeset.substring(0, 12) + 
            '</a>'; 
          //Create buglink
          re = new RegExp("[bB][uU][gG]\s\d{6}");
          var bugInDesc = (pushData[i].desc).search(re); 
          if(bugInDesc != -1) {
            var bugLinkName = (pushData[i].desc).substring(bugInDesc, bugInDesc + 10);
            var bugNumber = bugLinkName.substring(4, 10);
            var bugLink = 
              (pushData[i].desc).substring(0, bugInDesc) + 
              '<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=' + 
              bugNumber + 
              '">' + 
              bugLinkName + 
              '</a>' + 
              (pushData[i].desc).substring(bugInDesc + 10, (pushData[i].desc).length);
          } else { //No bug provided
              var bugLink = pushData[i].desc;
          }
          var tdScrollAuthorDesc = document.createElement("td");
          tdScrollAuthorDesc.innerHTML += '<strong>' + pushData[i].author + ' &mdash ' + bugLink + '</strong>';
          trScroll.appendChild(tdScrollUser);
          trScroll.appendChild(tdScrollChangeset);
          trScroll.appendChild(tdScrollAuthorDesc);
          loader.innerHTML = "";
          document.getElementById("titlePush").appendChild(trScroll);
 
          //Check whether it is a merge changeset or not
          if(pushData[i].MergeData != []) {
            for(var j = 0; j < pushData[i].mergeData.length; j++) {
              if(pushData[i].mergeData[j] != "") {
                var mergeStr = pushData[i].mergeData[j];
                for(var k = 0; k < pushData[i].mergeData[j].length; k++) {
                  var actualMergeStr = mergeStr[k].split('|-|');
                  var mergeC = actualMergeStr[0];
                  var mergeUser = actualMergeStr[1];
                  var mergeDesc = actualMergeStr[2];
                  if(mergeDesc != pushData[i].desc) {
                    var trScrollMerge = document.createElement("tr");
                    trScrollMerge.style.backgroundColor = trScroll.style.backgroundColor;
                    var tdScroll_MergeUser = document.createElement("td");
                    tdScroll_MergeUser.width = "184px";
 
                    //Create changset link 
                    var tdScroll_MergeC = document.createElement("td");
                    tdScroll_MergeC.innerHTML +=
                      '<a href="/rev/' +
                      mergeC.substring(0, 12) + 
                      '">' + 
                      mergeC.substring(0, 12) +
                      '</a>';    
                    //Create buglink
                    reg = new RegExp("[bB][uU][gG]\s\d{6}");
                    var merge_bugInDesc = mergeDesc.search(reg);
                    if(merge_bugInDesc != -1) {
                      var merge_bugLinkName = mergeDesc.substring(merge_bugInDesc, merge_bugInDesc + 10);
                      var merge_bugNumber = merge_bugLinkName.substring(4, 10);
                      var merge_bugLink =
                        mergeDesc.substring(0, merge_bugInDesc) + 
                        '<a href="https://bugzilla.mozilla.org/show_bug.cgi?id=' + 
                        merge_bugNumber + 
                        '">' + 
                        merge_bugLinkName + 
                        '</a>' + 
                        mergeDesc.substring(merge_bugInDesc + 10, mergeDesc.length);
                    } else { //No bug provided
                        var merge_bugLink = mergeDesc;
                    }
                    var tdScroll_MergeAuthorDesc = document.createElement("td");
                    tdScroll_MergeAuthorDesc.innerHTML += '<strong>' + mergeUser + ' &mdash ' + merge_bugLink + '</strong>';
                    trScrollMerge.appendChild(tdScroll_MergeUser);
                    trScrollMerge.appendChild(tdScroll_MergeC);
                    trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
                    document.getElementById("titlePush").appendChild(trScrollMerge);
                  }
                }
              }
            }
          }
        }    
      } 
    }
  }
  pushCheckins.send(null);
}

I found that the bugLink code was being repeated. I was using it to create bugLinks for both normal and merge changesets. I decided to move the bugLink code to a function to stop the code repetition. By doing this I was able to reduce the number of lines of code by 27 lines (see below).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88

function renderMorePushLogResults() {
  var end = start;
  start = start - 20
  var pushCheckins = new XMLHttpRequest();
  pushCheckins.open('GET', '/json-pushes?startID=' + start + '&endID=' + end, true);
  pushCheckins.onreadystatechange = function() {
    if(pushCheckins.readyState == 4)  {
      if(pushCheckins.status != 404) {
        var loader = document.getElementById("loader");
        loader.innerHTML = '<img src="{url}static/ajax-loader.gif" align="right" />';
        var pushData = new Function("return " + pushCheckins.responseText) ();
        var counter = 0;
        for(var i = end; i > start; i--) {
          var trScroll = document.createElement("tr");
          if(counter == 0) {
            trScroll.className = "parity0"; 
            counter = 1;
          } else {
              counter = 0;
              trScroll.className = "parity1";
          }      
          var tdScrollUser = document.createElement("td");
          tdScrollUser.width = "184px";
          tdScrollUser.innerHTML += pushData[i].user + '<br />' + pushData[i].formattedDate;
          //Create changset link
          var tdScrollChangeset = document.createElement("td");
          tdScrollChangeset.innerHTML += 
            '<a href="/rev/' + 
            pushData[i].individualChangeset.substring(0, 12) + 
            '">' + 
            pushData[i].individualChangeset.substring(0, 12) + 
            '</a>'; 
 
          //Create bug link
          var bugLink = createBuglink(pushData[i].desc);
 
          var tdScrollAuthorDesc = document.createElement("td");
          tdScrollAuthorDesc.innerHTML += '<strong>' + pushData[i].author + ' &mdash ' + bugLink + '</strong>';
          trScroll.appendChild(tdScrollUser);
          trScroll.appendChild(tdScrollChangeset);
          trScroll.appendChild(tdScrollAuthorDesc);
          loader.innerHTML = "";
          document.getElementById("titlePush").appendChild(trScroll);
 
          //Check whether it is a merge changeset or not
          if(pushData[i].MergeData != []) {
            for(var j = 0; j < pushData[i].mergeData.length; j++) {
              if(pushData[i].mergeData[j] != "") {
                var mergeStr = pushData[i].mergeData[j];
                for(var k = 0; k < pushData[i].mergeData[j].length; k++) {
                  var actualMergeStr = mergeStr[k].split('|-|');
                  var mergeC = actualMergeStr[0];
                  var mergeUser = actualMergeStr[1];
                  var mergeDesc = actualMergeStr[2];
                  if(mergeDesc != pushData[i].desc) {
                    var trScrollMerge = document.createElement("tr");
                    trScrollMerge.style.backgroundColor = trScroll.style.backgroundColor;
                    var tdScroll_MergeUser = document.createElement("td");
                    tdScroll_MergeUser.width = "184px";
 
                    //Create changset link 
                    var tdScroll_MergeC = document.createElement("td");
                    tdScroll_MergeC.innerHTML +=
                      '<a href="/rev/' +
                      mergeC.substring(0, 12) + 
                      '">' + 
                      mergeC.substring(0, 12) +
                      '</a>'; 
 
                    var merge_bugLink = createBuglink(mergeDesc);
 
                    var tdScroll_MergeAuthorDesc = document.createElement("td");
                    tdScroll_MergeAuthorDesc.innerHTML += '<strong>' + mergeUser + ' &mdash ' + merge_bugLink + '</strong>';
                    trScrollMerge.appendChild(tdScroll_MergeUser);
                    trScrollMerge.appendChild(tdScroll_MergeC);
                    trScrollMerge.appendChild(tdScroll_MergeAuthorDesc);
                    document.getElementById("titlePush").appendChild(trScrollMerge);
                  }
                }
              }
            }
          }
        }    
      } 
    }
  }
  pushCheckins.send(null);
}

Reduced lines of code needed to achieve the same functionality also reduces the chance of bugs occurring, which is always a good thing.

This entry was posted in DPS911, Mercurial Project, Open Source and tagged , , , . Bookmark the permalink.

One Response to v0.4 Release – Optimizing Client Side Code

  1. David Humphrey says:
    January 17, 2009 at 10:41 am

    Deleting code? Nice. Keep it up!

    Reply

Leave a Reply

Your email address will not be published. Required fields are marked *

CAPTCHA Image

*

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>