Can someone have a look to my Chess Project?

Question

I am currently working on an unfinished project about Chess, it will (at some point) be able to help me to make some chess variations just by pasting the FEN position while I play online.

But before continuing, my code must have good structure or it will become a nightmare to fix it later. So now that I have some basics, it would be great to have it reviewed:

//Note: the code uses jQuery

function writeBoard() {
    var i, j, str, bol;

    str = "<table id=\"chessboard\" cellpadding=\"0\" cellspacing=\"0\"><tbody>";
    for (i = 9; --i;) {
        str += "<tr>";
        for (j = 0; j < 8; j++) {
            bol = !bol;
            str += "<td class=\"" + (bol ? "ws" : "bs") + "\" id=\"" + (AbcLabels[j] + "" + i) + "\"></td>";
        }
        bol = !bol;
        str += "</tr>";
    }
    str += "</tbody></table>";

    $("body").append(str);
}

function setFEN(fen) {
    var i, j, len, arr, tmp, tmp2, tmp3;

    arr = (fen ? fen : DefaultFen).split(" ")[0].split("/");
    for (i = 0; i < 8; i++) {
        tmp = i * 8;
        for (j = 0, len = arr[i].length; j < len; j++) {
            tmp2 = arr[i].charAt(j);
            if (tmp2 * 1) {
                tmp += tmp2 * 1;
            } else {
                tmp3 = tmp2.toLowerCase();
                ChessBoard[i][tmp % 8] = PiecesNames.indexOf(tmp3) * (tmp2 != tmp3 ? 1 : -1);
                tmp++;
            }
        }
    }

    refreshBoard();
}

function refreshBoard() {
    var i, j, emt, fen, tmp, tmp2, tmp3, tmp4;

    fen = "";
    for (i = 9; --i;) {
        emt = 0;
        for (j = 0; j < 8; j++) {
            tmp = Math.abs(i - 8);
            tmp2 = ChessBoard[tmp][j] || 0;
            if (tmp2) {
                if (emt) {
                    fen += "" + emt;
                    emt = 0;
                }

                tmp3 = $("#" + AbcLabels[j] + "" + i);
                if (tmp2 < 0) {
                    tmp4 = PiecesNames.charAt(tmp2 * -1);
                    tmp3.addClass("b" + tmp4);
                    fen += tmp4;
                } else {
                    tmp4 = PiecesNames.charAt(tmp2);
                    tmp3.addClass("w" + tmp4);
                    fen += tmp4.toUpperCase();
                }
            } else {
                emt++;
            }
        }

        if (emt) {
            fen += "" + emt;
        }
        fen += "/";
    }

    console.log(fen); //remove the last "/"
    //fen+= can enPass, to move, castle Avility, clock, etc...
    //Fen=fen
}

var I, J, Len;
var AbcLabels = ["a", "b", "c", "d", "e", "f", "g", "h"];
var PiecesNames = "*pnbrqk"; //[wp:1][wn:2][wb:3][wr:4][wq:5][wk:6]***[bp:-1][bn:-2][bb:-3][br:-4][bq:-5][bk:-6]
var DefaultFen = "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq -";
var ChessBoard = new Array(8);
for (I = 8; I--;) {
    ChessBoard[I] = new Array(8);
}
var Fen;
var ToMove; //[0:white][1:black]
var WCastling, BCastling; //[0:none][1:short][2:long][3:both]
//var IsCheck,EnPassant;

$(function() {
    writeBoard();
    setFEN("5Rk1/1b2p2p/p2pP1p1/2rP4/2BQ2P1/6qP/PP6/1K6 b - -");
});

If you don't have a good text editor, you will find tmpX variables very hard to recognize one over the another... In my case, I just double click and they all get highlighted (using Notepad++).

Answer 1

The first thing I noticed are the escaping orgies. Both HTML and JavaScript allow you to use either ' or " to quote strings. Don't use the same for both and you can get rid of all those ugly backslashes.

You can also get rid of the (badly-named) bol variable. You already have a loop variable so you can use j % 2 == 0 or even simpler j & 1 to determine if the number is even or odd.

Answer 2

Not a big java person, but I'd say rename all your variables, especially the tmp ones. When you come back in 6 months, are you really going to know what tmp2, or fen is representing? Try coming up with more descriptive names, even if the name is longer, in the long run you'll thank yourself.

Answer 3

First obvious point: In JavaScript it's custom to give variables names starting with lower case letters.

One thing I would do is remove the "writing" of the board from the JavaScript and use a static HTML table instead.

You could remove both the class and the id from the table cells. The checkered color can easily be created with CSS, e.g.

#chessboard tr:nth-child(2n) > td:nth-child(2n+1), #chessboard tr:nth-child(2n+1) > td:nth-child(2n) {
    background-color: black;
}​

and the cells can be accessed using the rows and cells properties of the table DOM.

Prefilling the variable tmp (the file) with i * 8 doesn't mnake much sense, considering you do tmp % 8 anyway.

I'm not a big fan of displaying the pieces with CSS alone leaving the table practically empty. You should consider filling the cells with the appropriate Unicode characters and use the usual CSS text replacement techniques, if you want to display nicer images.

Is there a any special reason you are writing the setup to a global variable (ChessBoard) before filling the table? Global variables is something one always should avoid when programming. And splitting up both steps just requires you to run though the loops twice. The only advantage I see in spitting the two steps up is the separation of concerns (model, view), which doesn't seem to be a priority here.

Speaking of which, the "encoding" of the pieces in the ChessBoard as positive and negative numbers is quite cryptic. If you want to reuse that information, you may want to use something more readable, such as an object. Or at least document in a comment what you are doing.

The identification of digits by multiplication with one isn't very common, and should also be commented. Using the unitary plus operator is more common, and explicitly testing with ìsNaN would make it more readable:

if ( !isNaN(+tmp2) ) {

Finally fen ? fen : DefaultFen can be shorted to fen || DefaultFen

Here's how I would do it: http://jsfiddle.net/xL9B8/4/