patternjavascriptMinor
Updating Grid on Webpage (new version)
Viewed 0 times
webpagenewgridversionupdating
Problem
Previous version:
Updating Grid on Webpage
Task:
Draw a grid with a given number of rows and columns. Each cell can be
any color. The same grid should also be updated at a predetermined
time interval. The grid should cover the entire visible portion of the
page in the browser. Add a context menu with which you can change the
parameters of the lattice.
I would really like to hear feedback on the code: syntax, logic and general any comments and tips.
I'm a young programmer so I want to hear real reviews for further development.
Corrected:
HTML:
JS:
```
function Grid(gridOptions) {
this.numRows = gridOptions.numRows;
this.numColumns = gridOptions.numColumns;
this.updateInterval = gridOptions.updateInterval;
var clientWidth = document.body.clientWidth;
var clientHeight = document.body.clientHeight;
var interval=0;
var checkInterval;
var stopUpdateGrid = false;
this.createGrid = function(){
var container = document.createElement("div");
document.body.appendChild(container);
container.className="container";
for (var i = 1; i <= this.numRows * this.numColumns; i++) {
var cell = document.createElement("div");
container.appendChild(cell);
cell.className="cell";
cell.style.width = (clientWidth - this.numRows - 1) / this.numRows + "px";
cell.style.height = (clientHeight - this.numColumns - 1) / this.numColumns + "px";
var n = randomColors(
Updating Grid on Webpage
Task:
Draw a grid with a given number of rows and columns. Each cell can be
any color. The same grid should also be updated at a predetermined
time interval. The grid should cover the entire visible portion of the
page in the browser. Add a context menu with which you can change the
parameters of the lattice.
I would really like to hear feedback on the code: syntax, logic and general any comments and tips.
I'm a young programmer so I want to hear real reviews for further development.
Corrected:
- object passed to the constructor with parameters
- variables moved inside the constructor grid
- function
createGridas a method of object inside the constructor
- use style CSS
- added function that keeps track of dynamic resizing the browser window
HTML:
window.onload = function () {
var options = {
numRows : 20,
numColumns : 6,
updateInterval : 2000
}
var grid = new Grid (options);
}
JS:
```
function Grid(gridOptions) {
this.numRows = gridOptions.numRows;
this.numColumns = gridOptions.numColumns;
this.updateInterval = gridOptions.updateInterval;
var clientWidth = document.body.clientWidth;
var clientHeight = document.body.clientHeight;
var interval=0;
var checkInterval;
var stopUpdateGrid = false;
this.createGrid = function(){
var container = document.createElement("div");
document.body.appendChild(container);
container.className="container";
for (var i = 1; i <= this.numRows * this.numColumns; i++) {
var cell = document.createElement("div");
container.appendChild(cell);
cell.className="cell";
cell.style.width = (clientWidth - this.numRows - 1) / this.numRows + "px";
cell.style.height = (clientHeight - this.numColumns - 1) / this.numColumns + "px";
var n = randomColors(
Solution
Overall it looks pretty clean,
- I'd like to see some comments on the functions so I know what they do without having to read the code, you could use JSDoc for this.
- I'd recommend explicitly using Design By Contract or Defensive Programming so the users of the code know what to expect. Which one of these you choose will determine what you put in the comments.
- Get the tabbing fixed on the CSS file.
- Use constants where you have code like this:
e.keyCode === 13to make the meaning clearer.
- Ideally pull the text that is shown to the user out into a language file that defines appropriate variables, so that if you want to port to another language, you only have to send that to the translator, not the code.
Context
StackExchange Code Review Q#43751, answer score: 6
Revisions (0)
No revisions yet.