HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

MS Excel type of grid in jQuery

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
typejquerygridexcel

Problem

I have created an MS Excel type of grid in jQuery. I want to learn best practices and I need your comments for more optimized code.

Please review the code and offer your suggestions.

Demo

jQuery:

`var JS = JS || {};

JS.training = JS.training || {};
JS.training.tableData = JS.training.tableData || {};
JS.training.tableData = {

defaults: {
$table: $('#myTable'),
addTableRowBtn: $('#addRowBtn'),
addTableColBtn: $('#addColBtn')
},
createTable: function () {
var _this = this,
table = "";
table += "";
for (i = 0; i " : "A" + i + "";
table += tableHeader;
}
table += "";
table += "";
for (i = 0; i ";
for (var j = 0; j " + (i + 1) + "" : " ";
table += tableDataCells;
}
table += '';
}
table += "";
//APPEND TABLE MARKUP
$(_this.defaults.$table).append(table);
//BIND EVENTS
_this.bindEvents();
},
addTableRow: function () {
var _this = this,
colLen = $("#myTable tr:nth-child(1) td").length,
colVal = parseInt($("#myTable tr:last-child td:first").text()) + 1;
for (i = 0; i ";
for (var j = 0; j ' + colVal + ' ';
} else {
table += " ";
}
}
table += '';
}
$(_this.defaults.$table).append(table);
},
addTableColumn: function () {
var _this = this,
colVal = $("#myTable tr th:last-child").text(),
colNum = parseInt(colVal.charAt(1)) + 1;
console.log(colNum);
$("#myTable thead tr:last-child").append("A" + colNum + "");
$("#myTable tbody tr").each(function () {
$(this).append("")
});
},
bindEvents: function () {
var _this = this;
//CAPTURE ADD ROW BUTTON CLICK
_this.defaults.addTableRowBtn.on('click', fun

Solution

From a once over:

  • The column and row count should be a parameter/variable/default



  • The namespace JS.training seems like overkill, I doubt anybody would ever use 2 libraries for showing datagrids ;)



-
The following could use extraction of the pattern, also you should look into a templating library I am using my personal template function here as an example:

for (i = 0; i " : 
        "A" + i + "";
        table += tableHeader;
    }


could be

function fillTemplate( s )
{ //Replace ~ with further provided arguments
  for( var i = 1, a = s.split('~'), s = '' ; i ~`    
for (i = 0; i < 5; i++) {
  var header = (i == 0) ? "" : i;
  table += fillTemplate( headerTemplate, header );
}


-
I would split up the creation of header and items HTML into 2 separate functions

  • var _this = this is really only useful for closures, and you are not using those in createTable.



  • You should extract ` into a template as well, you use it both in createTable and addTableRow.



  • Same goes for the header template which you use as well in addTableColumn`



-
These comments can be filed under Captain Obvious and removed:

//CAPTURE ADD ROW BUTTON CLICK
_this.defaults.addTableRowBtn.on('click', function () {
    _this.addTableRow();
});

//CAPTURE ADD COLUMN BUTTON CLICK
_this.defaults.addTableColBtn.on('click', function () {
    _this.addTableColumn();
});


All in all I really like your code, I will leave it to @kleinfreund to review your CSS.

Code Snippets

for (i = 0; i < 5; i++) {
        var tableHeader = (i == 0) ? 
        "<th style='border:1px solid #E5E5E5; background:#F1F1F1'></th>" : 
        "<th style='border:1px solid #E5E5E5; background:#F1F1F1'>A" + i + "</th>";
        table += tableHeader;
    }
function fillTemplate( s )
{ //Replace ~ with further provided arguments
  for( var i = 1, a = s.split('~'), s = '' ; i < arguments.length ; i++ )
    s = s + a.shift() + arguments[i];
  return s + a.join("");
}     

var headerTemplate = `<th style='border:1px solid #E5E5E5; background:#F1F1F1'>~</th>`    
for (i = 0; i < 5; i++) {
  var header = (i == 0) ? "" : i;
  table += fillTemplate( headerTemplate, header );
}
//CAPTURE ADD ROW BUTTON CLICK
_this.defaults.addTableRowBtn.on('click', function () {
    _this.addTableRow();
});

//CAPTURE ADD COLUMN BUTTON CLICK
_this.defaults.addTableColBtn.on('click', function () {
    _this.addTableColumn();
});

Context

StackExchange Code Review Q#41130, answer score: 7

Revisions (0)

No revisions yet.