CODEWITHSUNDEEP
javascriptswitch-statement
paatryk01
paatryk01

Reputation: 19

Is it sensible to use switch in this case?

Is it sensible to use a switch in this situation ? Because after code review I get a comment to use a switch. Other question is, what should I change here to refactor this part of method ?

if(this.isGameOver) return;
        if(square.classList.contains('checked') || square.classList.contains('flag')) return
        if(square.classList.contains('bomb')) {
            this.gameOver();
        } else {
            let total = square.getAttribute('data');

            if(total != 0) {
                square.classList.add('checked');
                if(total == 1) square.classList.add('one');
                if(total == 2) square.classList.add('two');
                if(total == 3) square.classList.add('three');
                if(total == 4) square.classList.add('four');
                square.innerHTML = total;
                return
            }
            this.checkSquare(currentId);
        }
        square.classList.add('checked');

Upvotes: 0

Views: 64

Answers (4)

paatryk01
paatryk01

Reputation: 19

Some ideas how to refactor this ? It's recursion used in minesweeper to check every square around clicked square.

checkSquare(currentId) {

        const isLeftEdge = (currentId % this.width === 0);
        const isRightEdge = (currentId % this.width === this.width - 1);

        setTimeout(() => {
            
            if(currentId > 0 && !isLeftEdge) {
                const newId = this.cells[parseInt(currentId) - 1].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare)
            }

            if(currentId > 9 && !isRightEdge) {
                const newId = this.cells[parseInt(currentId) + 1 - this.width].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare);
            }

            if(currentId > 9) {
                const newId = this.cells[parseInt(currentId) - this.width].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare);
            }

            if(currentId > 11 && !isLeftEdge) {
                const newId = this.cells[parseInt(currentId) - 1 - this.width].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare);
            }

            if(currentId < 99 && !isRightEdge) {
                const newId = this.cells[parseInt(currentId) + 1].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare);
            }

            if(currentId < 90 && !isLeftEdge) {
                const newId = this.cells[parseInt(currentId) - 1 + this.width].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare);
            }

            if(currentId < 88 && !isRightEdge) {
                const newId = this.cells[parseInt(currentId) + 1 + this.width].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare);
            }

            if(currentId < 90) {
                const newId = this.cells[parseInt(currentId) + this.width].id;
                const newSquare = document.getElementById(newId);
                this.clicked(newSquare);
            }
        }, 10)
    },

Upvotes: 0

C. Graham
C. Graham

Reputation: 358

When it says use a switch, it is talking about your 5 if condition statements using total.

While Barmar's suggestion to use an array would be preferable, the refactoring tool is suggesting you switch on total.

ex:

switch(total){
    case 1:
        square.classList.add("one");
        break;
    case 2:
        square.classList.add("two");
        break;
    case 3:
        square.classList.add("three");
        break;
    case 4:
        square.classList.add("four");
        break;
}

Edit - I'd actually use the following method. it's easier to read imo.

    var classArr = square.classList;
    if (classArr.contains('checked') || classArr.contains('flag')) {
        return;
    } else if (classArr.contains('bomb')) {
        this.gameOver();
        return;
    } else {
        var total = square.getAttribute('data');
        var translate = { 0: "Zero", 1: "one", 2: "two", 3: "three", 4: "four" };
        classArr.add('checked');
        classArr.add(translate[total]);
        square.innerHTML = total;
        this.checkSquare(currentId);
    }

Upvotes: 0

paatryk01
paatryk01

Reputation: 19

Should be like this ?

        const totalClasses = ['one', 'two', 'three', 'four'];

        if(total > 0 && total <= totalClasses.length) {
            square.classList.add('checked');
            square.classList.add(totalClasses[total-1])
            square.innerHTML = total;
            return
        }

Upvotes: 0

Barmar
Barmar

Reputation: 781721

I would use an array:

const total_classes = ["one", "two", "three", "four"];
if (total > 0 && total <= total_classes.length) {
    square.classList.add(total_classes[total-1]);
}

Upvotes: 3

Related Questions