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

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. — 200_success
– 200_success, Commented Nov 4, 2016 at 19:20

Mario Santini · Accepted Answer · 2016-11-04 19:30:21Z

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.

Stack Exchange Network

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

1 Answer 1

You must log in to answer this question.

Hot Network Questions

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

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions