Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Object-oriented code

###Object-oriented code TheThe OO code looks like a good start, though for a larger application, some separation of model and view logic may be necessary. Also, the constructor of the Building class is a bit long. I would recommend abstracting out the code to add elements into separate methods- a template might make that process simpler.

Properly accessing the DOM

###Properly accessing the DOM BeforeBefore accessing DOM elements, it would be wise to wait until the DOM has been loaded. EventTarget.addEventListener() can be used on document to wait for the DOMContentLoaded event.

document.addEventListener("DOMContentLoaded", function(event) {
 var cookies_display = document.getElementById("cookies");
 var cookies_produced_display = document.getElementById("cookies_produced");
 var buildings = document.getElementById("buildings");
 //rest of code using cookies_display, cookies_produced_display and buildings
});

For more tips about optimizations with Javascript interacting with the DOM, I recommend this article. It has some tips like:

  • Caching DOM lookups - while there appears to be only one Clicker instance, each time the constructor is called it gets two elements by Id. Those should be cached in variables - and perhaps assigned once the DOMContentLoaded event is triggered.
  • Adding new elements to a document fragment - Apparently the code in the Building constructor does something similar to this with the <span> element assigned to the area property.

###Object-oriented code The OO code looks like a good start, though for a larger application, some separation of model and view logic may be necessary. Also, the constructor of the Building class is a bit long. I would recommend abstracting out the code to add elements into separate methods- a template might make that process simpler.

###Properly accessing the DOM Before accessing DOM elements, it would be wise to wait until the DOM has been loaded. EventTarget.addEventListener() can be used on document to wait for the DOMContentLoaded event.

document.addEventListener("DOMContentLoaded", function(event) {
 var cookies_display = document.getElementById("cookies");
 var cookies_produced_display = document.getElementById("cookies_produced");
 var buildings = document.getElementById("buildings");
 //rest of code using cookies_display, cookies_produced_display and buildings
});

For more tips about optimizations with Javascript interacting with the DOM, I recommend this article. It has some tips like:

  • Caching DOM lookups - while there appears to be only one Clicker instance, each time the constructor is called it gets two elements by Id. Those should be cached in variables - and perhaps assigned once the DOMContentLoaded event is triggered.
  • Adding new elements to a document fragment - Apparently the code in the Building constructor does something similar to this with the <span> element assigned to the area property.

Object-oriented code

The OO code looks like a good start, though for a larger application, some separation of model and view logic may be necessary. Also, the constructor of the Building class is a bit long. I would recommend abstracting out the code to add elements into separate methods- a template might make that process simpler.

Properly accessing the DOM

Before accessing DOM elements, it would be wise to wait until the DOM has been loaded. EventTarget.addEventListener() can be used on document to wait for the DOMContentLoaded event.

document.addEventListener("DOMContentLoaded", function(event) {
 var cookies_display = document.getElementById("cookies");
 var cookies_produced_display = document.getElementById("cookies_produced");
 var buildings = document.getElementById("buildings");
 //rest of code using cookies_display, cookies_produced_display and buildings
});

For more tips about optimizations with Javascript interacting with the DOM, I recommend this article. It has some tips like:

  • Caching DOM lookups - while there appears to be only one Clicker instance, each time the constructor is called it gets two elements by Id. Those should be cached in variables - and perhaps assigned once the DOMContentLoaded event is triggered.
  • Adding new elements to a document fragment - Apparently the code in the Building constructor does something similar to this with the <span> element assigned to the area property.
added 32 characters in body
Source Link

###Object-oriented code The OO code looks like a good start, though for a larger application, some separation of model and view logic may be necessary. Also, the constructor of the Building class is a bit long. I would recommend abstracting out the code to add elements into separate methods- a template might make that process simpler.

###Properly accessing the DOM Before accessing DOM elements, it would be wise to wait until the DOM has been loaded. EventTarget.addEventListener()EventTarget.addEventListener() can be used on document to wait for the DOMContentLoaded event.

document.addEventListener("DOMContentLoaded", function(event) {
 var cookies_display = document.getElementById("cookies");
 var cookies_produced_display = document.getElementById("cookies_produced");
 var buildings = document.getElementById("buildings");
 //rest of code using cookies_display, cookies_produced_display and buildings
});

For more tips about optimizations with Javascript interacting with the DOM, I recommend this article. It has some tips like:

  • Caching DOM lookupsCaching DOM lookups- while there appears to be only one Clicker instance, each time the constructor is called it gets two elements by Id. Those should be cached in variables - and perhaps assigned once the DOMContentLoaded event is triggered.
  • Adding new elements to a document fragment. I guessAdding new elements to a document fragment - Apparently the code in the Building constructor does something similar to this with the <span> element assigned to the area property.

###Object-oriented code The OO code looks like a good start, though for a larger application, some separation of model and view logic may be necessary. Also, the constructor of the Building class is a bit long. I would recommend abstracting out the code to add elements into separate methods- a template might make that process simpler.

###Properly accessing the DOM Before accessing DOM elements, it would be wise to wait until the DOM has been loaded. EventTarget.addEventListener() can be used on document to wait for the DOMContentLoaded event.

document.addEventListener("DOMContentLoaded", function(event) {
 var cookies_display = document.getElementById("cookies");
 var cookies_produced_display = document.getElementById("cookies_produced");
 var buildings = document.getElementById("buildings");
 //rest of code using cookies_display, cookies_produced_display and buildings
});

For more tips about optimizations with Javascript interacting with the DOM, I recommend this article. It has some tips like:

  • Caching DOM lookups- while there appears to be only one Clicker instance, each time the constructor is called it gets two elements by Id. Those should be cached in variables - and perhaps assigned once the DOMContentLoaded event is triggered.
  • Adding new elements to a document fragment. I guess the code in the Building constructor does something similar to this with the <span> element assigned to the area property.

###Object-oriented code The OO code looks like a good start, though for a larger application, some separation of model and view logic may be necessary. Also, the constructor of the Building class is a bit long. I would recommend abstracting out the code to add elements into separate methods- a template might make that process simpler.

###Properly accessing the DOM Before accessing DOM elements, it would be wise to wait until the DOM has been loaded. EventTarget.addEventListener() can be used on document to wait for the DOMContentLoaded event.

document.addEventListener("DOMContentLoaded", function(event) {
 var cookies_display = document.getElementById("cookies");
 var cookies_produced_display = document.getElementById("cookies_produced");
 var buildings = document.getElementById("buildings");
 //rest of code using cookies_display, cookies_produced_display and buildings
});

For more tips about optimizations with Javascript interacting with the DOM, I recommend this article. It has some tips like:

  • Caching DOM lookups- while there appears to be only one Clicker instance, each time the constructor is called it gets two elements by Id. Those should be cached in variables - and perhaps assigned once the DOMContentLoaded event is triggered.
  • Adding new elements to a document fragment - Apparently the code in the Building constructor does something similar to this with the <span> element assigned to the area property.
Source Link

###Object-oriented code The OO code looks like a good start, though for a larger application, some separation of model and view logic may be necessary. Also, the constructor of the Building class is a bit long. I would recommend abstracting out the code to add elements into separate methods- a template might make that process simpler.

###Properly accessing the DOM Before accessing DOM elements, it would be wise to wait until the DOM has been loaded. EventTarget.addEventListener() can be used on document to wait for the DOMContentLoaded event.

document.addEventListener("DOMContentLoaded", function(event) {
 var cookies_display = document.getElementById("cookies");
 var cookies_produced_display = document.getElementById("cookies_produced");
 var buildings = document.getElementById("buildings");
 //rest of code using cookies_display, cookies_produced_display and buildings
});

For more tips about optimizations with Javascript interacting with the DOM, I recommend this article. It has some tips like:

  • Caching DOM lookups- while there appears to be only one Clicker instance, each time the constructor is called it gets two elements by Id. Those should be cached in variables - and perhaps assigned once the DOMContentLoaded event is triggered.
  • Adding new elements to a document fragment. I guess the code in the Building constructor does something similar to this with the <span> element assigned to the area property.
lang-js

AltStyle によって変換されたページ (->オリジナル) /