I’ve decided to make a small change for this release along with the other changes that I have already outlined. I want to avoid code duplication as much as possible and thus I have decided to put code to create a rev link into a function. Currently the code for creating a rev link looks like the following:
var tdScrollChangeset = document.createElement("td"); tdScrollChangeset.innerHTML += '<a href=\"/rev/' + pushData[i].individualChangeset.substring(0, 12) + '\">' + pushData[i].individualChangeset.substring(0, 12) + '</a>';
After my change the code looks like the following:
var tdScrollChangeset = createRevLink(pushData[i].individualChangeset);
function createRevLink(str) { var td = document.createElement("td"); td.innerHTML += '<a href=\"/rev/' + str.substring(0, 12) + '\">' + str.substring(0, 12) + '</a>'; return td; }
This alteration reduces the number of lines of code but keeps the same functionality. A win win by all accounts.
#1 by Justin Wood (Callek) at March 15th, 2009
I didn’t note this on bug, but as a NIT: perhaps a rename of createRevLink to createRevTd(rev) would make sense.
Letting the attrib indicate what to pass in, and the function name indicating what it returns, which is _not_ an anchor element like I thought when skimming your code.
#2 by Justin Wood (Callek) at March 15th, 2009
s/attrib/argument/ from my last comment.