Skip to main content
1 of 2
RomanPerekhrest
  • 5.3k
  • 1
  • 9
  • 21

Toward restructuring and optimization

The expressions Math.floor(event.clientX / 60) and Math.floor(event.clientY / 60) which represents element position are redundantly duplicated across a half of the entire posted function's content.
The Extract function technique is reasonably applied and expressed with a separate function:

function getElPosition(e){
    return {x: Math.floor(e.clientX / 60),
            y: Math.floor(e.clientY / 60)};
}

hasClicked and highlightPos variables are assigned with same values in both exclusive branches of if/else conditional.
Thus, the assignment statements are moved out as common ones.
Furthermore, both highlightPos and advancePosition are essentially point to the target event element position returned by mentioned getElPosition function. Therefore, they could be just eliminated (see the full approach below).


The condition:

if (pieceMoves.moves[i].x - 2 == highlightPos.x || pieceMoves.moves[i].x + 2 == highlightPos.x) {
    isCastling = true;
} else {
    isCastling = false;
}

is just a verbose version of:

isCastling = (pieceMoves.moves[i].x - 2 == elPos.x || pieceMoves.moves[i].x + 2 == elPos.x);

Noisy condition:

if (
    board[highlightPos.y][highlightPos.x] == whiteKingSideCastle ||
    board[highlightPos.y][highlightPos.x] == whiteQueenSideCastle ||
    board[highlightPos.y][highlightPos.x] == blackKingSideCastle ||
    board[highlightPos.y][highlightPos.x] == blackQueenSideCastle ||
    board[highlightPos.y][highlightPos.x] == blackKing ||
    board[highlightPos.y][highlightPos.x] == whiteKing
)

is replaced with flexible Array.includes feature:

if ([whiteKingSideCastle, whiteQueenSideCastle, blackKingSideCastle, 
     blackQueenSideCastle, blackKing, whiteKing].includes(board[elPos.y][elPos.x]))

The can_move flag can be eliminated and replaced with check of pieceMoves.length.


See the full optimized approach:

function getElPosition(e) {
    return {
        x: Math.floor(event.clientX / 60),
        y: Math.floor(event.clientY / 60)
    };
}

document.addEventListener('click', function(event) {
    let elPos = getElPosition(event);
    if (!hasClicked) {
        if (event.clientX < 480 && event.clientY < 480 && board[elPos.y][elPos.x] != "vacant") {
            if (humanPlayer.indexOf(board[elPos.y][elPos.x]) != -1) {
                isHighlightPossibleMoves = true;
                pieceMoves = processMoves(Object.assign({}, elPos), board);
            }
            hasClicked = true;
        }
    } else {
        if (pieceMoves && pieceMoves.length) {
            for (let i = 0, len = pieceMoves.moves.length; i < len; i++) {
                if (elPos.x == pieceMoves.moves[i].x && elPos.y == pieceMoves.moves[i].y) {
                    if (board[elPos.y][elPos.x] == blackKing || board[elPos.y][elPos.x] == whiteKing) {
                        isCastling = (pieceMoves.moves[i].x - 2 == elPos.x || pieceMoves.moves[i].x + 2 == elPos.x);
                    }
                    if (!isCastling) {
                        if ([whiteKingSideCastle, whiteQueenSideCastle, blackKingSideCastle,
                                blackQueenSideCastle, blackKing, whiteKing
                            ].includes(board[elPos.y][elPos.x])) {
                            board[elPos.y][elPos.x].hasClicked = true;
                        }
                        board = chess.updateBoard(elPos, elPos);
                        chess = new Chess(board);
                        break;
                    }
                    board = chess.returnCastledBoard({
                        x: elPos.x,
                        y: elPos.y
                    }, pieceMoves.moves[i]);
                    chess = new Chess(board);
                    isCastling = false;
                }
            }
        }
        hasClicked = false;
        pieceMoves = undefined;
    }
});
RomanPerekhrest
  • 5.3k
  • 1
  • 9
  • 21