3
\$\begingroup\$

This was my first attempt at writing object-oriented JavaScript. It's a Minesweeper clone in jQuery. I thought I had done an OK job, but I've just received some feedback stating that it wasn't written in an object-oriented manner. I don't expect anyone to read this line for line. I'm just looking for general feedback.

$(document).ready(function() {
    var Minesweeper = function() {
            var self = this;
            var defaultOptions = {
                boardSize: [8, 8],
                difficulty: 'easy'
            }
            // initialize our cell object
            var cells;
            this.init = function(options) {
                // empty the cell object if it has anything in it
                cells = [];
                if (typeof options == 'object') {
                    options = $.extend(defaultOptions, options);
                } else {
                    options = defaultOptions;
                }
                self.difficulty = options.difficulty;
                // setup a new game
                self.newGame(options.boardSize);
            };
            this.newGame = function(boardSize) {
                //remove the old board if there is one, detach event listeners as well
                $('#minesweeper').empty();
                var x = boardSize[0];
                var y = boardSize[1];
                self.width = x;
                //setup our new board
                self.addUI();
                self.buildGrid(x, y);
            }
            this.addUI = function() {
                var board = $('#minesweeper');
                var checkMe = $('<div/>').attr({
                    id: 'check-me',
                    class: 'happy-smiley'
                });
                var mineCount = $('<div/>').attr('id', 'mine-count');
                var timer = $('<div/>').attr('id', 'timer').html('0');
                var ui = $('<div/>').attr('id', 'ui');
                ui.append(mineCount);
                ui.append(timer);
                ui.append(checkMe);
                board.append(ui);
            }
            this.buildGrid = function(columns, rows) {
                var rowDiv;
                var id = 0;
                // build a containg div for each row, and then populate each row with the correct
                // number of cells to fill out our grid
                for (var i = 0; i <= rows - 1; i++) {
                    rowDiv = $('<div/>').attr({
                        'id': 'row' + i,
                        'class': 'row'
                    }).appendTo($('#minesweeper'));
                    for (var j = 0; j <= columns - 1; j++) {
                        var cell = $('<div/>').attr({
                            'id': id++,
                            'class': 'cell'
                        }).appendTo(rowDiv);
                        // add the cell to the cells array so we can access it later instead of 
                        // looping through the DOM
                        cells.push({
                            'hasMine': false,
                            'flagged': false,
                            'cleared': false
                        });
                    }
                }
                // set the width and height of our minesweeper container based on how many columns we have 
                var cellWidth = $('.cell').outerWidth();
                var uiHeight = $('#ui').outerHeight();
                $('#minesweeper').width(cellWidth * columns).height(cellWidth * rows + uiHeight);
                $('.cell').css('cursor', 'pointer');
                // add event listeners for our newly created cells
                self.setHandlers();
                // add the mines to our grid 
                self.setMines(self.difficulty);
            }
            this.setMines = function(difficulty) {
                // the number of mines is different for each difficulty level
                switch (difficulty) {
                case 'hard':
                    var mineDensity = 4;
                    break;
                case 'middle':
                    var mineDensity = 5.2;
                    break;
                default:
                    var mineDensity = 6.4;
                    break;
                }
                var numberOfCells = cells.length;
                var maxCellId = numberOfCells - 1;
                var numberOfMines = Math.round(numberOfCells / mineDensity);
                self.mineCount = numberOfMines;
                $('#mine-count').html(numberOfMines);
                for (var i = 0; i < numberOfMines; i++) {
                    // generate a random number between 0 and the maximum cell id we have
                    var cellId = Math.floor(Math.random() * maxCellId) + 1;
                    // check if we have already placed a mine on this cell
                    // if we have, deincrement the counter to make sure we get the correct number of mines
                    if (cells[cellId].hasMine) {
                        i--;
                    } else {
                        // place mine
                        cells[cellId].hasMine = true;
                        $('#' + cellId).addClass('mine');
                    }
                }
            }
            this.setHandlers = function() {
                var firstClick = true;
                $('.cell').on('click', function() {
                    //start the timer only on the first click
                    if (firstClick) {
                        self.startTimer();
                        firstClick = false;
                    }
                    var id = this.id;
                    if (cells[id].hasMine) {
                        self.gameOver();
                        $(this).css('background-color', '#ff0000');
                    } else {
                        self.clearCell(id);
                    }
                });
                $('.cell').on('contextmenu', function(event) {
                    var cell = $(this);
                    var id = this.id;
                    if (cells[id].flagged) {
                        cell.removeClass('flagged');
                        cells[id].flagged = false;
                        cells[id].checked = false;
                        self.mineCount++;
                        $('#mine-count').html(self.mineCount);
                        return false;
                    } else {
                        if (cells[id].cleared) {
                            return false;
                        } else {
                            cell.addClass('flagged');
                            cells[id].flagged = true;
                            self.mineCount = self.mineCount - 1;
                            $('#mine-count').html(self.mineCount);
                            return false;
                        }
                    }
                    event.preventDefault();
                });
                $('#check-me').on('click', function() {
                    if ($(this).hasClass('sad-smiley')) {
                        self.reset();
                    } else {
                        self.check();
                    }
                });
            }
            this.gameOver = function() {
                $('.cell').css('cursor', 'default');
                $('#minesweeper .mine').toggleClass('show');
                $('#check-me').attr('class', 'sad-smiley');
                //lock the game board so the user can't click on any cells 
                $('.cell').off();
                self.stopTimer = true;
            }
            this.cheat = function() {
                $('.mine').toggleClass('show');
                setTimeout(function() {
                    $('.mine').toggleClass('show');
                }, 1000);
            }
            this.reset = function() {
                self.init();
            }
            this.check = function() {
                var loser;
                $.each(cells, function(index) {
                    if (cells[index].cleared == false && cells[index].hasMine == false) {
                        self.gameOver();
                        alert('Looks like you missed a few...');
                        loser = true;
                        return false;
                    }
                });
                if (!loser) {
                    self.win();
                }
            }
            this.win = function() {
                $('#check-me').attr('class', 'cool-smiley');
                alert("Winner!")
            }
            this.getNeighbors = function(cellId) {
                var neighbors = new Array();
                var neighborBombs = 0;
                var width = self.width;
                // need to fix this. It was just a hack to make sure we had an integer not a string
                var id = +cellId;
                // If we are on the top row, the id - width will always be < 0
                if (id - width > 0) {
                    // We are not on the top row. So get the cell above us (Top)
                    var top = id - width;
                    neighbors.push(top);
                    if (cells[top].hasMine) {
                        neighborBombs += 1;
                    }
                    // before we get the Top Right cell, check that we aren't on the far right edge of the board
                    if (((top + 1) % width) !== 0) {
                        var topRight = top + 1;
                        neighbors.push(topRight);
                        if (cells[topRight].hasMine) {
                            neighborBombs += 1;
                        }
                    }
                    // do the same with the Top Left cell
                    if ((top % width) !== 0) {
                        var topLeft = top - 1;
                        neighbors.push(topLeft);
                        if (cells[topLeft].hasMine) {
                            neighborBombs += 1;
                        }
                    }
                }
                // if the cell id is evenly divisble by the width, then we are on the left edge
                if (id % width !== 0) {
                    var left = id - 1;
                    neighbors.push(left);
                    if (cells[left].hasMine) {
                        neighborBombs += 1;
                    }
                }
                // if the cell id + 1 is evenly divisible by the width, then we are on the right edge
                if ((id + 1) % width !== 0) {
                    var right = id + 1;
                    neighbors.push(right);
                    if (cells[right].hasMine) {
                        neighborBombs += 1;
                    }
                }
                // if the cell id + the width of the grid is < the total number of cells, we are not on the bottom row
                if (id + width < cells.length) {
                    var bottom = id + width;
                    neighbors.push(bottom);
                    if (cells[bottom].hasMine) {
                        neighborBombs += 1;
                    }
                    // check that we aren't on the right edge of the board
                    if ((bottom + 1) % width !== 0) {
                        var bottomRight = bottom + 1;
                        neighbors.push(bottomRight);
                        if (cells[bottomRight].hasMine) {
                            neighborBombs += 1;
                        }
                    }
                    // check that we aren't on the left edge of the board
                    if (bottom % width != 0) {
                        var bottomLeft = bottom - 1;
                        neighbors.push(bottomLeft);
                        if (cells[bottomLeft].hasMine) {
                            neighborBombs += 1;
                        }
                    }
                }
                var cellData = {
                    cells: neighbors,
                    bombs: neighborBombs
                }
                return cellData;
            }
            this.startTimer = function() {
                self.stopTimer = null;
                var time = 0;
                $('#timer').html(time);
                (function tick() {
                    if (self.stopTimer) {
                        clearTimeout(moreTime);
                    } else {
                        time += 1;
                        $('#timer').html(time);
                        var moreTime = setTimeout(tick, 1000);
                    }
                })();
            }
            this.clearCell = function(cellId) {
                if (cells[cellId].checked) {
                    // been here before...
                } else {
                    // make sure there isn't flag on this cell before we clear it
                    if (cells[cellId].flagged == false) {
                        $('#' + cellId).addClass('cleared');
                        cells[cellId].checked = true;
                        cells[cellId].cleared = true;
                        var neighbors = self.getNeighbors(cellId);
                        if (neighbors.bombs > 0) {
                            $('#' + cellId).html(neighbors.bombs);
                            $('#' + cellId).addClass("number" + neighbors.bombs);
                        } else {
                            $('#' + cellId).addClass('cleared');
                            $.each(neighbors.cells, function(index, value) {
                                if (cells[value].hasMine) {} else {
                                    self.clearCell(value);
                                }
                            });
                        }
                    } else {
                        cells[cellId].checked = true;
                    }
                }
            }
        }
    var minesweeper = new Minesweeper();
    minesweeper.init({
        boardSize: [8, 8],
        difficulty: 'easy'
    });
    // add event listeners for the options panel
    $('#cheat').on('click', function() {
        console.log('cheated');
        minesweeper.cheat();
    });
    $('#new-game').on('click', function() {
        minesweeper.reset();
        minesweeper.stopTimer = true;
    });
    $('#difficulty').change(function() {
        minesweeper.init({
            'difficulty': $(this).val()
        });
    });
    $('#board-size').change(function() {
        var selected = $(this).find('option:selected');
        var x = selected.data('x');
        var y = selected.data('y');
        minesweeper.init({
            boardSize: [x, y]
        });
    })
    $('#show-timer').change(function() {
        $('#timer').toggleClass('hidden');
    });
});
$(window).load(function() {
    //preload images that will be needed later
    $.fn.preload = function() {
        this.each(function() {
            $('<img/>')[0].src = this;
        });
    }
    $(['/images/mine.png', '/images/red_flag.png', '/images/smiley_cool.png', '/images/smiley_happy.png', '/images/smiley_sad.png']).preload();
});

Any thoughts or improvements? Other feedback I have received was that my getNeighbors method was too complicated (which it probably is), but I was purposefully trying not to use a coordinate type system. My mistake, I guess.

\$\endgroup\$
  • \$\begingroup\$ Your entire program is pretty much in one function, try separating it into objects and methods. Also the usage of self is confusing, the language already has the perfectly fine keyword this \$\endgroup\$ – Esailija Aug 15 '13 at 0:55
  • \$\begingroup\$ Thanks for the feedback. I used self in order to access 'this' inside of the method calls, but maybe it's not necessary. Relevant SO question about it stackoverflow.com/questions/3309516/…. \$\endgroup\$ – FajitaNachos Aug 15 '13 at 1:00
  • \$\begingroup\$ Just saw your other comment. Thanks for the helpful feedback. \$\endgroup\$ – FajitaNachos Aug 15 '13 at 1:04
  • 8
    \$\begingroup\$ I see, well try to identify the objects in your program. The physical entities or abstract concepts. I can recognize plenty of objects here: Cell, Board, Game, Grid, Mine, Timer and so on. Then design their methods and interactions with each other, and model them with classes (constructors and prototypes). Don't just cram everything inside one function because I have to scroll horizontally because the indentation level is so much out of control. \$\endgroup\$ – Esailija Aug 15 '13 at 1:04
5
\$\begingroup\$

As @Esailija has said in the comments, OOP is about, well, objects.

Before you write any code, you should stop and think - what are the key concepts at play here? This is a Minesweeper implementation, so the very first thing you should do is lay down all the objects you can think of, what their respective members might be, and how they're all related.

As you write your code, you should keep an eye out for objects and methods that do too many things. For your code to be simple, each building block must also be fairly simple, i.e. if an object owns too much of the functionality, part of it can probably be broken down into simpler objects.

After you implemented an object, don't just move on to the next one: re-read your code, refactor repeating or redundant code into smaller reusable functions, lookout for cyclomatic complexity (e.g. nested ifs / "arrow" code) and review all names involved. Proper naming is just as important as getting the code to work: if it's a pet project you might finish it and then forget it, but in real-world code you're going to have to maintain the code, and that means all you've written was about 20% of the time you'll end up spending on that code - the other 80% is reading your code. That said your naming looks good :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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