5
\$\begingroup\$

I am practically brand new to coding and this is my first "real" project I've been attempting. It's an attempt at a vanilla JS game with selectable layout and win conditions (partially implemented). ATM it only has a vertical win condition included as I didn't want too much cluttering of the code (horizontal and diagonal would look more so the same).

SCRIPT.JS

const mainDiv = document.querySelector('[data-screen]')

class TicTacToe {
    constructor(div) {
        this.board = div.querySelector('[data-board]') 
    }

    //most of the initial variables here
    varUpdater = () => {
        this.pauseButton = this.board.querySelector('[data-pause-button]')
        this.pauseButton.addEventListener('click', this.setUp)
        this.pauseScreen = this.board.querySelector('[data-pause]')
        this.pauseText = this.board.querySelector('[data-pause-text]')

        this.playerOne = "x"
        this.playerTwo = "o"
        this.currentPlayer = this.playerOne

        this.winCondition = 3 //adjusts win condition

        this.rows = 5  // these values adjust layout
        this.columns = 5
    }

    setUp = () => {
        this.pauseMenuSetUp()
        this.varUpdater()
        this.cellSetUp()
        this.start()
    }

    pauseMenuSetUp = () => {
        this.board.innerHTML = `
        <div class="pause-screen show" data-pause>
            <div data-pause-text></div>
            <button data-pause-button>Start</button>
        </div>
        `
    }

    //Sets up cell layout and css style based on given parameters
    cellSetUp = () => {
        this.board.style.cssText = `grid-template-columns: repeat(${this.columns}, auto);`
        this.colSeperatorDiv = document.createElement('div')
        this.cellElement = document.createElement('div')
        this.cellElement.setAttribute('class', 'cell')
        for (let i = 0; i < this.rows; i++){
            this.cellElement.setAttribute('data-cell', i)
            this.colSeperatorDiv.appendChild(this.cellElement.cloneNode(true))
        } 
        for (let i = 0; i < this.columns;i++){
            this.colSeperatorDiv.setAttribute('data-seperator', i)
            this.board.appendChild(this.colSeperatorDiv.cloneNode(true))
        }
    }

    //Clears cell attributes and adds event listener before removing pause screen
    start = () => {
        this.board.querySelectorAll('[data-cell]').forEach(cell => {
            cell.classList.remove('x', 'o')
            cell.removeEventListener('click', this.clickHandle)
            cell.addEventListener('click', this.clickHandle, { once: true })
        })
        this.pauseScreen.classList.remove('show')
    }

    //Adds symbol based on cell target
    clickHandle = (cell) => {
        this.cell = cell.target
        this.cell.classList.add(this.currentPlayer)
        this.checkBoard()
    }

    //Decides if it should select next player or conclude game
    checkBoard = () => {
        this.columnIndex = parseInt(this.cell.getAttribute('data-cell'))
        this.rowIndex = parseInt(this.cell.parentElement.getAttribute('data-seperator'))
        if (this.isConcluded() == true) {
            this.pauseText.innerHTML = `${this.currentPlayer} wins!`
            this.pauseScreen.classList.add('show')
        } else if (this.isConcluded() === 'tie') {
            this.pauseText.innerHTML = `Tie!`
            this.pauseScreen.classList.add('show')
        } else {
            this.playerSelect()
        }
    }

    //Selects next player
    playerSelect = () => {
        this.currentPlayer !== this.playerTwo ? this.currentPlayer = this.playerTwo
            : this.currentPlayer = this.playerOne;
    }

    //Checks for game conclusion. Does NOT check for horizontal and diagonal conditions atm
    isConcluded = () => {
        return this.verticalScore(0, this.columnIndex) >= this.winCondition ? true
        : Array.from(this.board.querySelectorAll('[data-cell]')).every(c => c.classList.contains(this.playerOne) || c.classList.contains(this.playerTwo)) ? 'tie'
        : false
    }

    //Goes down the column using recursion to add up score,
    //Diagonal/horizontal win condition would use the rowIndex as well
    verticalScore = (score, col) => {
        return this.cell.parentElement.querySelector(`[data-cell="${col}"]`) == null ? score += this.reverseDirectionScore(0, this.columnIndex - 1)
        : this.cell.parentElement.querySelector(`[data-cell="${col}"]`).classList.contains(this.currentPlayer) ? score += this.verticalScore(1, col + 1)
        : score += this.reverseDirectionScore(0, this.columnIndex - 1);
    }
    //Goes up column using recursion to return a score
    reverseDirectionScore = (rScore, rCol) => {
        return this.cell.parentElement.querySelector(`[data-cell="${rCol}"]`) == null ? rScore
        : this.cell.parentElement.querySelector(`[data-cell="${rCol}"]`).classList.contains(this.currentPlayer) ? rScore += this.reverseDirectionScore(1, rCol - 1)
        : rScore
    }
}

const newBoard = new TicTacToe(mainDiv)
newBoard.setUp()

attaches to this HTML page

<!DOCTYPE html>
<html>
<head>
    <title>Tic-Tac-Toe</title>
    <link rel="stylesheet" href="styles.css">
</head>
<body>
    <div data-screen>
        <div class="board" data-board>
        </div>
    </div>
    <script src="/script.js" async defer></script>
</body>

</html>

Codepen: https://codepen.io/sarbastu/pen/XWjZgey

The game is functioning as intended, but I have a bunch of doubt on the way it was written particularly with possible overuse of querySelector/data-attributes. A few particular questions:

  1. Was the choice to make the game a template object pointless/more damaging than otherwise?
  2. Does this code at all represent decently implemented OOP style?
  3. How is the use of querySelector (mainly the use of it in the last 3 game conclusion check functions)? Should I have attached them to a variable first? Is there anything I could have done better?

Do to my complete lack of experience I have no idea if I am at all approaching OOP or programming in general correctly. Any criticism/feedback is greatly appreciated.

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

As you are as you say "brand new" to coding this will be less like a review and more like some pointers and advice..

Some pointers

  • Rather than define the functions as arrow functions within the class verticalScore = (score, col) => { Use the shorter syntax verticalScore(score, col) {

    There is a technical reason due to the way this is associated with arrow functions differently from standard function (arrow functions close over this while standard function are bound to this)

  • JavaScript actually requires that expressions be terminated with semicolons. If you dont add them the parser will try an guess where they should be. 99% of the time it gets it right, but when it does not your code will not work as expected, or throw an error and it will not be obvious looking at the code what is wrong.

    To be safe always add the semicolon ; If you are not sure add it. Its better to have too many than trying to fix the JavaScript's parser bad guess.

  • JavaScript was designed to be easy to learn and use and had many features that in hindsight turned out to make life hard for those new to code. To help fix this problem they added strict mode that will highlight problematic code with errors.

    As a beginner it is best to always add the directive "use strict"; at the very top (first line) of the JS file. It will save you hours of bug hunting due to problems that a few simple rules will avoid.

  • Go easy on the comments. Things like //Selects next player followed by the line playerSelect = () => { does not make a good comment and just clutters up the code.

    As a beginner comments help you keep track of your thought patterns as you code. Thus when adding comment think of them a future notes to self, do you really need them. In the long run good code does not need comments, good code explains its self.

Objects

OOP does not mean using objects per say. As a beginner you should focus on objects as abstract entities with roles and responsibilities. OOP is a very advanced subject and the nitty details can often elude even experienced coders.

"Does this code at all represent decently implemented OOP style?"

You have defined an object with a properties and functions. You have instantiated an instance of that object const newBoard = new TicTacToe(mainDiv) and then called its functions, so you have the basics.

OOP is not a style, it is in the structure, logic, and organisation of the code.

For now keep using the basics, and as you get comfortable you will naturally begin to expand you knowledge of OOP

Developing your use of objects

Objects should be focused to specific parts of the task. You have one Object that does everything. Consider some creating more objects to do specific parts of the job.

Some object could be...

  • The game state has a start, play, gameover cycles, show the text Game Over, Player won, that is a task for an object.

  • There are two players that have turns. An object to store the players, whos turn is it, is it a draw, who won, who lost.

  • An object to hold a player, name, what side they are X or O, Track wins and losses.

  • You have cells that make up the board. An object to handle all the cells, with functions like reset, checkWin, isValidMove etc.

  • Each cell can also be an object, it handles the rendering and visual appearance of the cell, providing functions to allow other objects know what the cell is doing (empty, X or O), where the cell is. Etc

All these objects define the various abstract concepts that make up the game. If done well then the higher and more abstract the object, the more you code using these abstractions.

For example at the top level Object you may call Game, it holds the board, players, and game state. It manages the various lower level objects. It would instruct the board object to reset game.board.reset() or this.board.reset(). Call game.players.nextUp() to get the next player set up to play with the correct pieces (X, O). and so on. Call game state to display a new game is starting game.gameState.start(game.players.currentTurn) passing it the player that has the first turn.

How you break the task up into abstract concepts and associated objects is up to you, what is best is a skill to learn. One object to do all things is the wrong path to good OOP

But be warned, breaking a task into to too many parts can make the task of managing them all become larger and more complex that the problem you are solving (namely play a game). Objects should make things easier, not harder.

Three questions

"How is the use of querySelector (mainly the use of it in the last 3 game conclusion check functions)?"

  1. Your use of querySelector is poor. See answer to next question.

"Should I have attached them to a variable first?"

  1. Yes and no. Looking at your code you have not shown any detailed use of arrays. The cells of the board should be stored in an array and encapsulated by an object. Often in tic-tac-toe people use a 2D array. Detailing how to use arrays is a little outside the scope of CR answers.

    MDN provides many resources and are a good place to get a start in JS. For more on arrays. There are also many other resources there to help you get started as a coder

"Is there anything I could have done better?"

  1. Again.. Yes and sort of.

    • The Yes The first part of this code review highlights some of the more important things to do better. As for the rest there is a long list of things that could be better, many of these points are still hotly debated amongst professional coders. More often than not better is a personal preference. Could you do better, yes, everyone can always do better.

    • The sort of Does the code work? Have you tried your best to use game in a way that breaks it? Did you encounter bugs and did you fix them? If you answer yes to these questions then you have done very well.

      Coding is hard, some people will never be coders, others take to it like ducks to water. Though your the code is not up to professional standards getting it to work bug free without fear of a user doing the wrong thing at the wrong time is a clear sign that you have what is needed to be a great coder.

      There is a lot to learn, and you must master the basics of problem solving using the features of the language you choose to start with. There is only one way to do that and that is by gaining experience as you code.

Template object?

"Was the choice to make the game a template object pointless/more damaging than otherwise?"

I am not that sure what you mean by template object, something I am unfamiliar with apart from its literal meaning.

If you mean that the defined object TicTacToe is a template for other games then yes pointless, not damaging.

Conclusion

As a beginner you have done well, and if you enjoy coding then do keep at it, because enjoying something that can also be a career is a priceless asset to have.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.