ES6 class that generates an area where you can draw on a page

Question

I have the following javascript es6 class (along with some functions) that generates an area where you can draw in a page.

class Painter {
 constructor(selector, cols, rows, style) {
 this.looper = null;
 if (!selector)
 throw new TypeError("Provide a valid table selector");
 this.cols = cols || 20;
 this.rows = rows || 20;
 this.selector = selector;
 this.style = style;
 let markup = "";
 for (let height = 0; height < this.rows; height++) {
 markup += "<tr>";
 for (let width = 0; width < this.cols; width++)
 markup += "<td data-col=" + width + " data-row=" + height + "></td>";
 markup += "</tr>";
 }
 $(function() {
 $(selector).html(markup);
 $(selector).on("click", "td", function() {
 $(this).toggleClass("tile-active");
 });
 });
 }
 setStyleSheet(stylesheet) {
 this.style = stylesheet;
 }
 changeTileColor(stylesheet, newColor) {
 if (!newColor)
 return;
 var css = ".tile-active{\n\tbackground:" + newColor + ";\n}";
 try {
 changeCss((stylesheet || this.style), css);
 } catch (e) {
 console.log("Suppressed error");
 }
 }
 createChessBoard() {
 var id = this.selector;
 $(function() {
 $(id + " tr:nth-child(even) td:nth-child(even)").addClass("black-tile");
 $(id + " tr:nth-child(odd) td:nth-child(odd)").addClass("grey-tile");
 });
 }
 toggleBorder() {
 var bname = $(this.selector).css("border");
 $(this.selector).css("border", bname.indexOf("none") != -1 ? "2px solid" : "none");
 }
 clearBoard() {
 $(this.selector + " .tile-active").removeClass("tile-active");
 }
 createRandowDraw() {
 this.clearBoard();
 let mat = $(this.selector + " td");
 let max = mat.length;
 for (let i = 0; i < max; i++) {
 let num = Math.floor(Math.random(2) * 2);
 if (num && num <= 1)
 $(mat[i]).addClass("tile-active");
 }
 }
}

As I am new to classes in ECMAScript 6 I was wondering if there is anything I can do to improve the performance/readability of this code.

Here is a link to the following code: Painter

• 200_success: Your question says that this code "generates an area where you can draw in a page", but the code seems to have a lot to do with tiles and chessboard patterns. Could you clarify? Ideally, make a live demo (press Ctrl-M in the question editor) so that we can see exactly what is going on.

Answer 1

The first weird thing I saw in your code is the use of the $(fun).

This jQuery function is used to execute code when the DOM is ready.

I think it should be better to remove all this calls and instead instantiate the class itself in a function executed after the DOM is ready.

That helps to follow the code properly.

// here you declare the class
class MyClassThatUseDOM {
 constructor(selector) {
 this.myElement = $(selector);
 }
}
$(function(){
 // here you instantiate the class 
 // when the DOM is ready and all selector
 // will properly match.
 const myObj = new MyClassThatUseDOM("#theselector");
});

As the element $(selector) or $(this.selector) are used may times it is best to save in an instance variable:

this.element = $(selector);

And then access always from this variable, instead of continue to search it with jQuery.

The following method have some issue for me:

createChessBoard() {
 var id = this.selector;
 $(function() {
 $(id + " tr:nth-child(even) td:nth-child(even)").addClass("black-tile");
 $(id + " tr:nth-child(odd) td:nth-child(odd)").addClass("grey-tile");
 });
}

Maybe if you remove the $(func) part as I suggested in my first point the code will be more clear.

But let's discuss what we have now.

You use a variable id to save the selector, but id is more specific term than selector. It could confuse some developer.

A selector for jQuery is anything is a valid css selector, like a tag name, a css class name or even and id.

Please pay attention to the names you use for your variables.

In this case you can do something like:

let _selector = this.selector

As you can see you understand that you just assign a private variable, here.

Use of var is not wrong, but we have let.

Similar considerations for:

var bname = $(this.selector).css("border");

About the method:

createRandowDraw() {
 this.clearBoard();
 let mat = $(this.selector + " td");
 let max = mat.length;
 for (let i = 0; i < max; i++) {
 let num = Math.floor(Math.random(2) * 2);
 if (num && num <= 1)
 $(mat[i]).addClass("tile-active");
 }
}

Here you could use jQuery API to simplify your code:

createRandowDraw() {
 this.clearBoard();
 $(this.selector + " td").each(function(i, mat){
 let num = Math.floor(Math.random(2) * 2);
 if (num && num <= 1) {
 $(mat).addClass("tile-active");
 }
 });
}

Or in a plain javascript fashion by using filter and forEach.