# Battle Noughts vs Crosses in Javascript

This simple game tries to solve the problem of a game that often end in a lock. It does so by providing the players with Xs and Os with superpowers. With great powers come possible disadvantages. For example the X- can delete an O already on the board, but because the effect contains a random element, it is just as likely to remove one of your own pieces. Therefore, a player must manage risk, in addition to other normal strategies.

The game is a two player game at the moment and the interface is very simple. But it works. A few minor bugs I haven't figured how fix don't affect the gameplay. An example is that the "Player 'x' has won the game" alert fires off before the game piece is drawn on the board. I have also hard coded the coordinates for game winning checks. I am sure there is an easier (algorithmic) way of doing that.

var currentPlayer = "O";
var currentType = "Noughts";
var currentAction = "picking";
var coordList = ["0_0", "1_0", "2_0", "0_1", "1_1", "2_1", "0_2", "1_2", "2_2"];
function place(box) {
//Verifies the box is empty
if(box.innerText != "") return;
//insert object current object into box
box.innerText = currentPlayer.substring(0,1);
//rules for alternatives
//+ Battle Rule
if(currentPlayer.substring(1,2) == "+") {
var attackOpt = [];

}
}

var fightingNumber = Math.floor(Math.random() * 6) + 1;

if(attackOpt.length != 0){
var opponentBox = attackOpt[Math.floor(Math.random() * attackOpt.length)];
if(fightingNumber => 4){
document.getElementById(opponentBox).innerHTML = box.innerText;
}
else {
box.innerText = document.getElementById(opponentBox).innerHTML
}
}

}

//- Remove Rule
if(currentPlayer.substring(1,2) == "-") {
if(delOpt.length != 0){
var delBox = delOpt[Math.floor(Math.random() * delOpt.length)];
document.getElementById(delBox).innerText = "";

}
}

if(currentPlayer.substring(1,2) == "÷") {

}
}

box.innerText = contents;

}
}
//* Dupicate Rule
if(currentPlayer.substring(1,2) == "*") {

if(cloneOpt.length != 0){
var cloneBox = cloneOpt[Math.floor(Math.random() * cloneOpt.length)];
document.getElementById(cloneBox).innerText = box.innerText;
}
}

//Flips and prepares for next player
currentType == "Noughts" ? currentType = "Crosses": currentType = "Noughts";
currentType == "Crosses" ? document.getElementById("PlayerChoise").innerHTML = '<input type="button" value="Ready Player Two? (X)" id="PlayerOne" onclick="showOptions()">':
document.getElementById("PlayerChoise").innerHTML = '<input type="button" value="Ready Player One? (O)" id="PlayerOne" onclick="showOptions()">';
//Checks for Winning Conditions
//Box 1

if(box.id == "0_0"){
checkGameBoard("0_0","1_0","2_0");
checkGameBoard("0_0","1_1","2_2");
checkGameBoard("0_0","0_1","0_2");
}
//Box 2
if(box.id == "1_0"){
checkGameBoard("0_0","1_0","2_0");
checkGameBoard("1_0","1_1","1_2");
}
//Box 3
if(box.id == "2_0"){
checkGameBoard("0_0","1_0","2_0");
checkGameBoard("2_0","1_1","0_2");
checkGameBoard("2_0","2_1","2_2");
}
//Box 4
if(box.id == "0_1"){
checkGameBoard("0_1","1_1","2_1");
checkGameBoard("0_0","0_1","0_2");
}
//Box 5
if(box.id == "1_1"){
checkGameBoard("0_0","1_1","2_2");
checkGameBoard("1_0","1_1","1_2");
checkGameBoard("2_0","1_1","0_2");
checkGameBoard("0_1","1_1","2_1");
}
//Box 6
if(box.id == "2_1"){
checkGameBoard("0_1","1_1","2_1");
checkGameBoard("2_0","2_1","2_2");
}
//Box 7
if(box.id == "0_2"){
checkGameBoard("0_0","0_1","0_2");
checkGameBoard("0_2","1_1","2_0");
checkGameBoard("0_2","1_2","2_2");
}
//Box 8
if(box.id == "1_2"){
checkGameBoard("0_2","1_2","2_2");
checkGameBoard("1_0","1_1","1_2");
}
//Box 9
if(box.id == "2_2"){
checkGameBoard("0_2","1_2","2_2");
checkGameBoard("2_2","1_1","0_0");
checkGameBoard("2_0","2_1","2_2");
}

currentAction = "picking";
}
currentX = parseInt(currentBox.substring(0,1));
currentY = parseInt(currentBox.substring(2,3));

if(currentX == 0) {
currentXOptions = [currentX, currentX + 1];
}
if(currentX == 1) {
currentXOptions = [currentX, currentX + 1, currentX -1];
}
if(currentX == 2) {
currentXOptions = [currentX, currentX - 1];
}

if(currentY == 0) {
currentYOptions = [currentY, currentY + 1];
}
if(currentY == 1) {
currentYOptions = [currentY, currentY + 1, currentY -1];
}
if(currentY == 2) {
currentYOptions = [currentY, currentY - 1];
}

for(x=0; x<currentXOptions.length; x++){
for(y=0; y<currentYOptions.length; y++){
}
}

}
else {
}
}

}
if(status == "full"){
}
else {
}
}

function checkGameBoard(firstCoord,secondCoord,thirdCoord) {
var first = document.getElementById(firstCoord).innerText;
var second = document.getElementById(secondCoord).innerText;
var thrid = document.getElementById(thirdCoord).innerText;
if(first == "") return;
if(first == second && first == thrid){
alert(currentPlayer.substring(0,1) + " is the winner")
// for(i=0; i<coordList; i++) {
//     document.getElementById(coordList[i]).innerText = currentPlayer.substring(0,1);
// }
}
}
//+ attacks a random adjacent oppenent peice ÷ trades places with random adjacent opopnet * places a second copy on empty adjacent square
var noughts = ["O", "O+","O-", "O÷", "O*"]
var crosses = ["X", "X+", "X-", "X÷", "X*"]

function showOptions() {
if(currentType == "Noughts") {
document.getElementById("PlayerChoise").innerHTML = "";
for(i=0; i<noughts.length; i++) {
document.getElementById('PlayerChoise').innerHTML += '<input type="button" value="'+ noughts[i] + '" onclick="chooseThis(\'' + noughts[i] + '\')">';
}
}
else {
document.getElementById("PlayerChoise").innerHTML = "";
for(i=0; i<crosses.length; i++) {
document.getElementById('PlayerChoise').innerHTML += '<input type="button" value="'+ crosses[i] + '" onclick="chooseThis(\'' + crosses[i] + '\')">';
}

}
}

function chooseThis(item) {
if(currentAction == "picking") {
currentPlayer = item;
if(currentType == "Noughts") {
for(index=0; index<noughts.length; index++){
if(noughts[index]== item){
noughts.splice(index,1)
}
}
}
else {
for(index=0; index<crosses.length; index++){
if(crosses[index]== item){
crosses.splice(index,1)
}
}
}
showOptions();
currentAction = "playing";
}
}
.home {
display: flex;
justify-content: center;
align-items: center;
flex-direction: column;
height: 98vh;
margin: none;
}
.row {
display: flex;
flex-direction: row;
}
.row div {
border: 1px solid;
height: 30px;
width: 30px;
}
<!DOCTYPE html>
<html>
<title>Noughts and Crosses</title>
<script src="xoxo.js"></script>
<meta charset="UTF-8">
<body>
<div class="home">
<div class="tittle">
<h1>O vs. X</h1>
</div>
<div class="row">
<div id="0_0" onclick="place(this)"></div>
<div id="1_0" onclick="place(this)"></div>
<div id="2_0" onclick="place(this)"></div>
</div>
<div class="row">
<div id="0_1" onclick="place(this)"></div>
<div id="1_1" onclick="place(this)"></div>
<div id="2_1" onclick="place(this)"></div>
</div>
<div class="row">
<div id="0_2" onclick="place(this)"></div>
<div id="1_2" onclick="place(this)"></div>
<div id="2_2" onclick="place(this)"></div>
</div>
<div id="PlayerChoise">
<input type="button" value="Ready Player One? (O)" id="PlayerOne" onclick="showOptions()">
</div>
<div class="instructions">
<p>+ battles a random adjacent opponent (loss will give box to oponent)</p>
<p>- deletes a random adjacent nought or cross</p>
<p>* clones current box into random empty adjacent box</p>
</div>
</div>
</body>
</html>

# Review

Welcome to Code review.

Interesting project, I have never seen Battle TicTacToe so created a review to see how it played. It got a little out of hand.

I have trimmed quite a lot and I normally do a rewrite of the code. But in this case I found that I had to make major changes, thus making the rewrite completely unrelated to the original code. There is not much to learn from such an extensive rewrite.

## Bugs

There are potential and hidden bugs in your code. Hidden bugs are bugs that break the intended logic but are not obviously apparent (they don't throw errors)

• ALWAYS define variables. You have undefined variable i and many more. Using variables without defining them means that they are globals. Though in this example the undefined variable i will not cause you a problem. It would, if you called a function that used i as well.

Example the following code will lock up the page and require you to close it manually, because the second function doTwoThings will change the global i to 2 when it returns meaning the calling loop will never reach 10

doTenThings();
function doTwoThings() {
for(i = 0; i < 2; i++){
console.log("Hi there");
}
}

function doTenThings() {
for(i = 0; i < 10; i++){
doTwoThings()
}
}


For learning JavaScript you should make a habit of adding the directive "use strict"; as the first line of any javascript file or <script> tag as it will throw an error when you use an undefined variable. If added to the above example it would not run.

• You have the line if(fightingNumber => 4) { (which is a new one for me). Lucky (or unlucky) it does not create a parsing error because its an arrow function and the statement (fightingNumber => 4) === true. I assume you meant if(fightingNumber >= 4) {

## Style

Good code style is the most important attribute of a good programmer. There are a zillion styles out there, and the arguments as to which is best get heated.

Which is best is up to the individual but there are some style rules that help avoid some common language specific problems.

• Use === rather than == and !== rather than !=.
• Make unchanging variables constants. Eg var coordList = [ should be const coordList = [
• Don't forget to add semicolon to the end of lines.
• Variables declared as var should be defined at the top of the function. Do not define them as needed. Learn the difference between let, const and var and use the appropriate type of variable.
• Don't add code in the markup (HTML). Eg onclick="place(this)" should be in the JavaScript using addEventListener

There are style rules that make code readable, these are the most argued rules and thus none are definitive

• Spaces between operators. examples

• a+b as a + b
• if(a===b){ as if (a === b) {
• [1,2,3,4]; as [1, 2, 3, 4];
• Don't use comments to remove code. Remove it if it's no longer needed.

• If you find yourself prefixing variables with the same name it is a good sign that that prefix should be an object. Eg you have currentPlayer, currentAction, and currentType, is better as const current = {action: "picking", type: "Noughts", player: "O"};

• Use constants to define magic numbers and strings.

• Define grouped constants by name. E.g. actions = {picking: 1, playing: 2}; then you check for the current action with current.action === actions.playing

## Consistent style

There is one style rule that everyone can at least agree upon, it is by far the most important you must master.

Be Consistent If you add spaces between operators, then always do it. If you don't like semicolons, then never use them. Inconsistent style makes looking for bugs so much harder.

## Code

Good code has the following attributes

1. Is simple.
2. Is granular (many short functions rather than a few long functions).
3. Is well organised (related functions keep together, variables declared together, magic numbers as constants),
4. Is efficient.

Most of these relate to readability and maintainability, but an eye for efficiency is also important as users of your software don't see code, they see the end product and slow inefficient code does not make for happy clients and they are always the most important reviewers of your software.

Many of the following points cross over, they are listed in the most appropriate category.

### Incorrect language use

• You incorrectly use the ternary currentType == "Noughts" ? currentType = "Crosses": currentType = "Noughts"; should be currentType = currentType == "Noughts" ? "Crosses" : "Noughts";

### Maintainability

• If you have many elements that you assign the same event handler, you should consider a single handler and use the event object to determine which item has been clicked.

• The function place is way to long and should be reduced into smaller functions.

• Don't over complicate the system. The game has 9 items and you are indexing them as 9 3 character strings "0_0" to "2_2" which means your code is full of strings. These can be replace via an index, 0-8, that can be used as an array indexes to lookup content as needed. (you can almost half the code size by using index rather than the coord string)

• If you have a set of if statements that are exclusive, use else. Or if you have large sets of if {} else if {} ... else {} consider using switch statements, or even use lookups.

• The function checkGameBoard does more than check the board for a win, it handles the win as well. Try to keep a functions role too just one thing. It would be better if it returned the game status and then the calling function can handle the result.

### Performance

• Use element.textContent rather than element.innerText as it is faster and does not force a reflow.

• DOM queries such as document.getElementById are very sloooowwww.... Locate them once and put them in variables so you don't have to interrogate the DOM each time you need access.

• Learn to use the DOM API to create and add elements. Using innerHTML is again very sloooowwww....

• Use CSS style rules to change content. You keep removing and adding option buttons. You should have the button already defined and use a class rule to hide and show them as needed rather than add new ones each move.

• You can improve efficiency by precalculating information and storing it. For example the function checkAdjecent does a complicated sequence of steps to find locations. These locations can be stored in an array const adjacent = [[1,3], [0,4,2], [1,5], [0,4,6], [1,3,5,7], [2,4,8], [3,7], [4,6,8], [5,7]]; where the cells (boxes) are numbered from 0 to 8. Thus the adjacent boxes for box 4 (center) is array adjacent[4], [1,3,5,7]

### Simplify

Every extra line, even every character is another point where an error can occur. When looking for bugs, every extra character and line is more check. In a mass of code it is difficult to find typos. To reduce the chance of bugs you should constantly be thinking of ways to reduce the complexity of the code, both logically and in terms of size.

Some points on simplifying.

• You check for a win on each cell (box), however a win is a win no matter which cell is set so you can simplify the win by checking for all wins in the function. Thus you don't need that long list of if(bos == "1_1") just one call and check all winning rows for any win.

• If you are getting one character from a string you can use brackets to index the character currentPlayer.substring(1,2) becomes currentPlayer[1]

• You generate a random number from 0-6 and add one to it const fightingNumber = Math.random() * 6 + 1 | 0;, then you test if that number is fightingNumber >= 4. You can skip adding one and test if the number is > 2. However if you think about it you are using the random number to pick a 50% chance for one action or the other which can be done with if (Math.random() < 0.5) { or if (Math.random() >= 0.5) {

• You test for not an empty array with if(array.length !== 0) however the number 0 (and -0) equate to false and other numbers are true so you can do the same with if (array.length) {

• Ok! So many questions. I guess one at the time. On the Bug I am afraid this is the first time that the global and local thing has clicked for me. Am I correct to assume that ANYTIME I do not put var in front of a variable it becomes global? If I did var i = 0; in the precedent line would that work. That is understanding that I should probably use more descriptive variable names. – Gabe Ruiz Dec 19 '18 at 22:22
• @GabeRuiz Yes you can define the as a var at the top of the function, you need only var i; as it is assigned a value in the for loop, or you can define it as a let in the for loop for(let i = 0;... – Blindman67 Dec 19 '18 at 22:28