11
\$\begingroup\$

This is my attempt at the Ultimate Tic-Tac-Toe code challenge.

It uses jQuery to build out the game grid and find the states of all the "buttons" (actually <div>s) when calculating whether or not someone has won an individual board or the entire game.

This version follows the recommendation that a filled board that is a draw does not count for either player.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" lang="en" xml:lang="en">
<head>
    <meta http-equiv="Content-type" content="text/html;charset=UTF-8" />
    <title>Ultimate Tic-Tac-Toe</title>
    <script type="text/javascript" src="http://code.jquery.com/jquery-1.11.0.min.js"> </script>
    <style type="text/css">
        body { text-align: center; }
        div { display: inline-block; }
        .grid { padding: 5px; border: solid 1px #300; }
        .current-grid { background-color: #ff0; }
        .button { background-color: #eee; width: 25px; height: 25px; margin: 5px;
                  text-align: center; vertical-align: middle; font-size: 25px; }
        .X-won { background-color: #cfc; }
        .O-won { background-color: #cdf; }
        .no-win { background-color: #ccc; }
        .X-selected { color: #080; }
        .O-selected { color: #008; }
        #game { position: absolute; left: 0px; right: 0px; }
    </style>
    <script type="text/javascript">
<!--
        var isXTurn = true;
        var currentGrid = -1;

        $(function() {
            $('body').append('<div id="game"></div>');
            for (var i = 0; i < 9; i++) {
                $('#game').append('<div class="grid" id="grid' + i + '"></div>');
                if (i % 3 == 2) {
                    $('#game').append('<br />');
                }
                for (var j = 0; j < 9; j++) {
                    $('#grid' + i).append('<div class="button" id="button' + i + '-' + j + '"></div>');
                    if (j % 3 == 2) {
                        $('#grid' + i).append('<br />');
                    }
                }
            }
            $('.button').click(function() {
                var parent = $(this).parent();
                var me = isXTurn ? 'X' : 'O';

                if ($(this).is('.X-selected, .O-selected') || parent.is('.X-won, .O-won') ||
                    (currentGrid >= 0 && parent.attr('id') != 'grid' + currentGrid)) {
                    return;
                }
                $(this).addClass(me + '-selected').html(me);
                if (checkForWin($(this), ('.' + me + '-selected'))) {
                    parent.addClass(me + '-won');
                    if (checkForWin(parent, ('.' + me + '-won'))) {
                        window.alert(me + ' won!');
                        resetGame();
                        return;
                    }
                    else if ($('.grid.X-won, .grid.O-won, .grid.no-win').length == 9) {
                        window.alert('The game is a draw!');
                        resetGame();
                        return;
                    }
                }
                else if (parent.children('.X-selected, .O-selected').length == 9) {
                    parent.addClass('no-win');
                }
                currentGrid = +($(this).attr('id').slice(-1));
                $('.grid').removeClass('current-grid');
                if ($('#grid' + currentGrid).is('.X-won, .O-won, .no-win')) {
                    currentGrid = -1;
                }
                else {
                    $('#grid' + currentGrid).addClass('current-grid');
                }
                isXTurn = !isXTurn;
            });

            function checkForWin(elem, match) {
                var index = +(elem.attr('id').slice(-1));
                var prefix = '#' + (elem.hasClass('button') ? elem.attr('id').slice(0, 8) : 'grid');
                // Check rows
                if (elem.nextUntil('br').add(elem.prevUntil('br')).filter(match).length == 2) {
                    return true;
                }
                // Check columns
                if ($(prefix + (index % 3)).add($(prefix + (index % 3 + 3))).add($(prefix + (index % 3 + 6))).filter(match).length == 3) {
                    return true;
                }
                // Check diagonals
                if ($(prefix + 0).add($(prefix + 4)).add($(prefix + 8)).filter(match).length == 3) {
                    return true;
                }
                if ($(prefix + 2).add($(prefix + 4)).add($(prefix + 6)).filter(match).length == 3) {
                    return true;
                }
                return false;
            }
        });

        function resetGame() {
            isXTurn = true;
            currentGrid = -1;
            $('div').removeClass('X-won O-won no-win current-grid X-selected O-selected');
            $('.button').html('');
        }
//-->
    </script>
</head>
<body>
    <h1>Ultimate Tic-Tac-Toe!</h1>
    <p><a href="#" onclick="resetGame()">Restart</a></p>
</body>
</html>

It also passes the W3C validators for HTML and CSS, which is a bonus warm fuzzy.

Improve this question
\$\endgroup\$
0

3 Answers 3

Reset to default
10
\$\begingroup\$

  • You're making a few DOM elements with jQuery, which is fine. However instead of nasty strings like this;

       $('#game').append('<div class="grid" id="grid' + i + '"></div>');

    Consider creating the elements and modifying attributes using the jQuery functions, and then appending them to the game board. 

    $('<div id="grid>')
  .addClass('grid' + i)
  .appendTo('#game');

    I find it's almost always more legible and easier to understand when you use appendTo rather than append.

  • You reuse a few jQuery selectors. It's always better to store the elements than to look them up from the DOM each time. var $game = $('#game');

  • Your reset function includes this gem: $('div'). This is one of the worst selectors possible! It will select every div on the page! Since you only want to select the ones in your game then use a selector with context: $('div', $game). Or better still, use a class.

Improve this answer
\$\endgroup\$
5
  • 1
    \$\begingroup\$ I didn't realize that you could pass in context with a selector, which made changing the code to support multiple games on one page much easier. I did pull out some of the reused jQuery selectors as well. I believe $(this) is still cheap so I didn't pull that out, but I could be wrong. I did strip out the closing tag from my appends, but I didn't feel that using appendTo instead of append was significantly more readable, nor modifying attributes after creation. That's just my personal preference, though. \$\endgroup\$
    – Cameron
    Feb 7, 2014 at 0:28
  • \$\begingroup\$ I've fixed many functions in our code to reuse a selector variable for exactly the optimization reason you specify. Do you happen to have a link that addresses this across functions? IOW, is it better to store the jQuery object for use later across UI events vs. looking it up on each click? \$\endgroup\$
    – David Harkness
    Feb 7, 2014 at 4:02
  • \$\begingroup\$ @DavidHarkness If you're going to reuse a selector a lot, then it's better to store it as a variable. Remember each time you call $('') jQuery has to search the DOM for the element. \$\endgroup\$
    – Jivings
    Feb 7, 2014 at 8:16
  • \$\begingroup\$ @Jivings Yes, I expect that outweighs the bytes required to maintain a reference to the selector. \$\endgroup\$
    – David Harkness
    Feb 7, 2014 at 9:50
  • \$\begingroup\$ @DavidHarkness Memory is not so much an issue as speed. \$\endgroup\$
    – Jivings
    Feb 12, 2014 at 7:16
10
\$\begingroup\$

From a once over:

  • I would use the HTML 5 tags, they are cleaner and more appropriate:
    <!DOCTYPE html>
    <html>
    Your code validates perfectly as HTML5
  • The UI setup probably deserves it's own function
  • There is no separation of concerns ( more specifically logic and UI ) in your click handler, also you are using the UI as your data model, which is iffy.
  • I love how short resetGame is
  • A lot of the numbers are hard coded, you seem so close to do any number of boards, if you were to polish this code further, I would suggest you look into that

For fun I made it run on jsbin : http://jsbin.com/rohe/3

Improve this answer
\$\endgroup\$
1
  • \$\begingroup\$ I've incorporated your recommendations, thanks a lot for the input. I understand the concern about using the UI as the data model, but because the UI and the state of the game are so tightly bound (there is very little in the model that doesn't have a 1-to-1 correspondence with a visual representation in the UI) that it seemed silly to separate the two where they could potentially drift out of sync. It does make the click handler more complex than I would like and if the game was more complex I think then it would be worth separating the two. \$\endgroup\$
    – Cameron
    Feb 7, 2014 at 0:21
6
\$\begingroup\$

Just about everything I pointed out on a review for the same challenge applies here (are you sure you didn't just copy this guy?).

Attaching an event handler to each and every button element is highly inefficient (and fairly common mistake made by jQuery programmers). Instead, you should be using event delegation. A single event handler attached to the board itself is all you need to catch events on the descendant elements.

Using JavaScript to generate markup is inefficient: the more you can generate up front, the better. Unless your intent was to let the users decide how many tiles the board should have...

Validating HTML is pretty easy to do when your page contains virtually no markup. Semantically speaking, the generated board rather poor. There's no reason to ever use a <br /> between block level elements. If you need to force wrapping (because you've made them inline-bock), adjust the width of the parent element instead.

Improve this answer
\$\endgroup\$
6
  • 2
    \$\begingroup\$ Generating with JS keeps it DRY, your claims of inefficiency run counter to actually running the code. \$\endgroup\$
    – konijn
    Feb 6, 2014 at 16:00
  • \$\begingroup\$ @konijn So you think that generating the markup via JavaScript is faster (or just as fast) as having the markup in the document? It is possible to by DRY by generating it on the server side. \$\endgroup\$
    – cimmanon
    Feb 6, 2014 at 19:03
  • \$\begingroup\$ @cimmamon, I am saying that a human could not tell the difference, we are talking about shaving off milliseconds. \$\endgroup\$
    – konijn
    Feb 6, 2014 at 19:32
  • \$\begingroup\$ @konijn You'll nitpick about one optimization recommendation, but not the other? The user is unlikely to notice the difference between a single event handler vs an event handler on every element (since there aren't enough elements to make the page feel slow), too. \$\endgroup\$
    – cimmanon
    Feb 6, 2014 at 20:34
  • \$\begingroup\$ ;) I was nitpicking on both actually. The generation also increases DRYness so I called that out specifically. \$\endgroup\$
    – konijn
    Feb 6, 2014 at 20:40

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.