Cleaner structure for Battleships code

The code provided is working fine, but I would like to see if there is room for improvement for the code provided:

```
function Ship (size, direction) {
this.coveredFields = [];
this.place = function (sizeY, sizeX) { // sizeX & sizeY: size of fields in both dimensions

// pick randomly within our limits
var locationX;
var locationY;
if (direction) {
locationX = Math.floor(Math.random() * (sizeX - 1 - size));
locationY = Math.floor(Math.random() * (sizeY - 1));
} else {
locationX = Math.floor(Math.random() * (sizeX - 1));
locationY = Math.floor(Math.random() * (sizeY - 1 - size));
}

// setting locations
for (var i = 0 ; i "+ stats;
document.body.appendChild(div);

// assigning the final values to the final vars
document.getElementById("hitArea").innerHTML = "Hit accuracy: "+(3/guesses);
document.getElementById("guessArea").innerHTML = "Guesses: "+(guesses);
}
if(hits%3==0){
countSunkShip++;
document.getElementById("shipArea").innerHTML = "Ships sunk: "+ countSunkShip;
}
}
else
{
matchTrue = 0;
}// END if which checks locations == clicked items
}
}
//check if the match was made
if(matchTrue == 1){
//insert ship image here
var img = document.createElement('img');
img.src = "ship3.png";
img.style.backgroundPosition = "center center";
this.appendChild(img);
}
else{
//insert miss image here
var img = document.createElement('img');
img.src = "arrow.png";
this.appendChild(img);
}

var node = d.target.parentNode.querySelector("td");

I would like to touch on a few points, some of which are minor and others that are big picture. I will start with the minor issues first.

I am not sure if you simply made a mistake when copying the code onto this site, but you have some indentation issues here:

function Ship (size, direction) {
    this.coveredFields = [];
   this.place = function (sizeY, sizeX) { // sizeX & sizeY: size of fields in both dimensions

And here:

for (var i = 0 ; i < size ; i++) {
    if (direction) {
        this.coveredFields.push(locationY * 10 + locationX + i)
    } else {
        this.coveredFields.push((locationY + i) * 10 + locationX)
    }
   }
}

As well as some other places. You also have inconsistent brackets after else statements here:

else
            {
                matchTrue = 0;
            }// END if which checks locations == clicked items

And here:

else{
        //insert miss image here
        var img = document.createElement('img');
        img.src = "arrow.png";
        this.appendChild(img);
    }

Finally there is not enough white space in some of your if statements such as in these two examples:

if(hits%3==0){
if(matchTrue == 1){

It is important to follow the language conventions for whatever language you are using, and it is also important to have proper indentation and white space. It might be easy for you to read and understand the code in its current form and at this moment in time, but it is hard for anyone else to read it with these kinds of problems, and you will be left wondering what is going on when you come back to the code at a later time.

I would recommend that you put the { bracket on the same line as the if statement, and have a tab on the next line. Whether you decide to follow this guideline, or some other guideline, it is important to be consistent throughout your code. I would also recommend more white space, like this if (hits % 3 == 0) {. It is simply more readable this way.

Now on to a big design issue. I see you declaring variables like this:

var ship1 = new Ship(3, true);
ship1.place(6,6);
ship1.coveredFields;

var ship2 = new Ship(3, false);
ship2.place(6,6);
ship2.coveredFields;

var ship3 = new Ship(3, true);
ship3.place(6,6);
ship3.coveredFields;

Any time that you have a variable name followed by a number, it is easy to tell that there are problems with the code. It would be much better to have the game designed in such a way that it can take a number of ships as input rather than hard coding them. This is a sign that you should change the organization of the code (possibly dramatically).

In addition to this, you have a large number of magic numbers in the code, such as these:

locationX = Math.floor(Math.random() * (sizeX - 1));
locationY = Math.floor(Math.random() * (sizeY - 1 - size));

this.coveredFields.push(locationY * 10 + locationX + i)
this.coveredFields.push((locationY + i) * 10 + locationX)

It might be obvious to you, or maybe even to someone else, what the 1 and the 10 are supposed to represent, but I for one am not totally sure. It is a good idea to eliminate all magic numbers in your code, and replace them with things like this:

td.style.width = "50px";
td.style.height = "50px";
var shipLength = 7;

This way it is immediately clear to you or to anyone else exactly what the numbers are supposed to be.

//call function to create grid(7,7)
createTable(6,6);

The above snippet is a good example of why your comments should explain the Why of your code, rather than the What. In this case, it looks like you changed the size of the grid from 7 to 6, so the comment is not accurate. If the comment is accurate, then it should instead say why a 7 is going to make a grid of size 6.

There are lots more issues related to the overall flow of the code. I had a hard time following the logic of the game with the way that you have it organized. I would recommend structuring it differently, though I do not have a specific suggestion in this regard. The bottom line is that the different pieces of code are located in places that makes it hard to immediately tell what is going on, and when.

StackExchange Code Review Q#62336, answer score: 5