Крестики-нолики двигателя в JavaScript


Я просто скопировал (на GitHub клон, не буквальный клон) кто-то в крестики-нолики и я изначально хотела сделать pull-запрос, но я в конечном итоге сделать слишком много изменений, код просто неузнаваемым сейчас.

вот это оригинальный репозиторий

Моя цель-не создание лучшей крестики-нолики двигателя (если идея крестики-нолики двигатель не смешно в первую очередь), и я признаю, что мои намерения рефакторинга опубликовал окончательный код для CodeReview все вместе. Так, я не хочу, чтобы рекомендации по улучшению, связанные с крестики-нолики-связанные вещи, я хочу, общие рекомендации структура кода, являются ли шаблоны я использую никакого смысла, или что-нибудь еще, что вы думаете, было бы лучше реализованы по-другому.

Вот код двигателя:

const TicTacToe = (function() {
  // returns the index of an element within its parent
  const elementIndex = function elementIndex(el) {
    return Array.from(el.parentNode.children).indexOf(el);
  };

  const checkMovesHasCombination =
    function checkMovesHasCombination(combination, moves) {
      return combination.every((num) => moves.includes(num));
    };

  /* returns the winning combinations for the specified table-size in a
  2d array. */
  const getWinningCombinations = function getWinningCombinations(tableSize) {
    return getHorizontalWinningCombinations(tableSize)
      .concat( getVerticalWinningCombinations(tableSize) )
      .concat( getDiagonalWinningCombinations(tableSize) );
  };

  const getHorizontalWinningCombinations =
    function getHorizontalWinningCombinations(tableSize) {
      const combinations = [];
      for (let i = 0; i < tableSize; i++) {
        const combination = [];
        for (let a = 0; a < tableSize; a++) {
          combination.push( a + (i * tableSize) );
        }
        combinations.push(combination);
      }
      return combinations;
    };

  const getVerticalWinningCombinations =
    function getVerticalWinningCombinations(tableSize) {
      const combinations = [];
      for (let i = 0; i < tableSize; i++) {
        const combination = [];
        for (let a = 0; a < tableSize; a++) {
          combination.push( i + (a * tableSize) );
        }
        combinations.push(combination);
      }
      return combinations;
    };

  const getDiagonalWinningCombinations =
    function getDiagonalWinningCombinations(tableSize) {
      const combinations = [[], []];
      for (let i = 0; i < tableSize; i++) {
        combinations[0].push( i + (i * tableSize) );
        combinations[1].push( (tableSize - 1 - i) + (i * tableSize) );
      }
      return combinations;
    };

  // generates an HTML table of the specified size (size X size)
  const generateTable = function generateTable(size) {
    const table = document.createElement('table');
    for ( let i = 0; i < size; i++ ) {
      const tr = table.appendChild( document.createElement('tr') );
      for ( let a = 0; a < size; a++ ) {
        tr.appendChild( document.createElement('td') );
      }
    }
    return table;
  };

  const mapPlayer = function mapPlayer(player) {
    const playerCopy = Object.assign(player);
    playerCopy.moves = [];
    return playerCopy;
  };

  /* calculates the 'num' attribute for a cell, which represents
  the cell's position in the grid. */
  const getCellPosition = function getCellPosition(el) {
    return elementIndex(el) + (
      elementIndex(el.parentNode) * el.parentNode.children.length
    );
  };

  const Cell = function Cell(el, num) {
    this.el = el;
    this.setNumAttribute(num);
    this.player = null;
  };

  Cell.prototype.getNumAttribute = function getNumAttribute() {
    return parseInt(this.el.getAttribute('data-num'));
  };

  Cell.prototype.setNumAttribute = function setNumAttribute(num) {
    this.el.setAttribute('data-num', num);
    return this.el;
  };

  Cell.prototype.select = function select(player) {
    // if not empty, return 'false'.
    if (this.player) {
      return false;
    }
    this.player = player;
    this.el.innerHTML = this.player.char;
    if ( this.player.className ) {
      this.el.classList.add( this.player.className );
    }
    return true;
  };

  Cell.prototype.clear = function clear() {
    // if already empty, return.
    if (!this.player) {
      return;
    }
    this.el.innerHTML = '';
    if ( this.player.className ) {
      this.el.classList.remove( this.player.className );
    }
    this.player = null;
  };

  /*
  * 'size': an integer representing the number of both the columns and rows of
  * the game grid.
  * 'players': an object that has a 'char' (specifying the symbol to mark cells
  * with for the player) and a 'className' property (specifying the className
  * to be added to cells occupied by a player).
  *
  */
  const TicTacToe = function TicTacToe(size, players) {
    if ( !Number.isInteger(size) ) {
      throw new Error('You have to pass an integer for the "size" argument.');
    } else if ( !Array.isArray(players) || !players.length >= 2 ) {
      throw new Error(
        `You have to pass an array that has at least 2 elements for the
        "players" argument.`
      );
    } else {
      players.forEach( (player) => {
        if ( !(typeof player.char === 'string') ) {
          throw new Error(
            `Each element in the "players" argument must have a string
            "char" property.`
          );
        }
      } );
    }

    this.size = size;
    this.players = players.map( (player) => mapPlayer(player) );
    this._playersInitial = players;

    this._winningCombinations = getWinningCombinations(this.size);
    this._gameResolved = false;

    this._table = generateTable(this.size);
    this._cells = Array.from( this._table.getElementsByTagName('td') )
      .map( (el) => new Cell( el, getCellPosition(el) ) );
    this._cells.clearAll = function clearAll() {
      this.forEach((cell) => cell.clear());
    };
    this._cells.allOccupied = function allOccupied() {
      return this.every( (cell) => cell.player );
    };

    this._selectHandler = null;
    this._turnHandler = null;
    this._winHandler = null;
    this._drawHandler = null;
  };

  // checks if a player has won, if they have, it returns that player's object.
  TicTacToe.prototype._checkForWin = function checkForWin() {
    return this.players.find(
      (player) => this._winningCombinations.some(
        (combination) => checkMovesHasCombination(
          combination, player.moves
        )
      ),
      this
    );
  };

  // checks if the game is a draw (draw == all cells occupied && no one has won)
  TicTacToe.prototype._checkForDraw = function checkForDraw() {
    return this._cells.allOccupied() && !this._checkForWin();
  };

  TicTacToe.prototype._resolveGame = function resolveGame() {
    const winner = this._checkForWin();
    if (winner) {
      if ( typeof this._winHandler === 'function' ) {
        this._winHandler(winner);
      }
      this._gameResolved = true;
    } else if ( this._checkForDraw() ) {
      if ( typeof this._drawHandler === 'function') {
        this._drawHandler();
      }
      this._gameResolved = true;
    }
  };

  TicTacToe.prototype._handlePlayerTurns = function _handlePlayerTurns() {
    this.players.push( this.players.shift() );
    if ( typeof this._turnHandler === 'function'
      && !this._gameResolved === true ) {
      this._turnHandler( this.players[0] );
    }
    return this.players[0];
  };

  TicTacToe.prototype.selectCell = function selectCell(el) {
    if ( this._gameResolved === true ) {
      return;
    }
    const cell = this._cells.find( (cell) => cell.el === el );
    if ( cell && cell.select(this.players[0]) ) {
      this.players[0].moves.push( cell.getNumAttribute() );
      this._resolveGame();
      if ( typeof this._selectHandler === 'function' ) {
        this._selectHandler( el, this.players[0] );
      }
      this._handlePlayerTurns();
    }
  };

  TicTacToe.prototype.getBoard = function getBoard() {
    return this._table;
  };

  TicTacToe.prototype.reset = function reset() {
    this._cells.clearAll();
    this.players = this._playersInitial.map(
      (player) => mapPlayer(player)
    );
    this._gameResolved = false;
  };

  TicTacToe.prototype.registerTurnHandler =
    function registerTurnHandler(handler) {
      this._turnHandler = handler;
    };

  TicTacToe.prototype.registerWinHandler =
    function registerWinHandler(handler) {
      this._winHandler = handler;
    };

  TicTacToe.prototype.registerDrawHandler =
    function registerDrawHandler(handler) {
      this._drawHandler = handler;
    };

  TicTacToe.prototype.registerSelectHandler =
    function registerDrawHandler(handler) {
      this._selectHandler = handler;
    };

  return TicTacToe;
})();

Вот как вы его используете:

  const players = [
    {
      char: '&#10005;',
      className: 'playerX',
      id: 1,
    },
    {
      char: '&#9711;',
      className: 'playerY',
      id: 2,
    },
  ];

  const ticTacToe = new TicTacToe(3, players);

  const containerElement = document.getElementById('container');
  const turnText = document.getElementById('playerTurn');
  const resetButton = document.getElementById('reset');

  const ticTacToeBoard = ticTacToe.getBoard();
  containerElement.insertBefore(ticTacToeBoard, resetButton);

  Array.from(ticTacToeBoard.getElementsByTagName('td'))
    .forEach(
      (el) => el.addEventListener(
        'click',
        (e) => ticTacToe.selectCell(e.target)
      )
    );


  ticTacToe.registerTurnHandler(
    (player) => {
      turnText.innerHTML =
        'It\'s ' + (player['id'] === 1 ? 'X' : 'O') + '\'s turn.';
    }
  );


  ticTacToe.registerWinHandler(
    (player) => turnText.innerHTML =
      (player['id'] === 1 ? 'X' : 'O') + ' won the game.'
  );

  ticTacToe.registerDrawHandler(
    () => turnText.innerHTML = 'Game Over! It\'s a draw.'
  );


  resetButton.addEventListener( 'click', () => {
    ticTacToe.reset();
    turnText.innerHTML = 'It\'s X\'s turn.';
  });

Красивая крестики-нолики игра, построенная на вершине выше код может быть найден здесь.
И вот это GitHub репозитории.

Выложил новую версию код (тот, который я обновил с принятыми ответа по рекомендации) на Reddit, чтобы получить комментарий о том, что как хорошо, вот это.



505
3
задан 24 февраля 2018 в 04:02 Источник Поделиться
Комментарии
1 ответ

Стены баги.

Ваш код не совсем хорошо. Это пример банан в ООП джунгли, набирает ни одно из преимуществ ООП и усилительных только плохое.

Вы раскрыли почти все состояние объекта, которое является общим в отношении принципов ООП.

Некоторые примечания.


  • Именованные выражения функции обладают свойствами, которые встречаются очень редкие потребности. Использование объявления функции. Например, использовать function name(){} не const name = function name(){}

  • Подчеркивает не защищают государство, не частные переменные, а не код.

  • Использовать геттеры и сеттеры например cell.prototype.getNumAttribute и cell.prototype.setNumAttribute должен иметь геттер и сеттер и падение шума

    Они становятся get num(){ return this.el.dataset.num } и set num(val) { this.el.dataset.num = val }


  • registerSelectHandler ???? Ваш код-это не отдел гов, он не имеет регистрации, а функции не выполняют действия, которые могут рассматриваться как регистрация, свою простую задачу. TicTacToe.prototype.registerSelectHandler = function registerDrawHandler(handler) {

    Использовать API,addEvent(name, func) {... и веть Функ, когда его добавил Не тогда, когда вы собираетесь ее назвать.

    Или использовать геттеры и сеттеры set onwin(func) {... и веть Функ


  • Не обратные вызовы непосредственно из кода. Они могут бросить, и ваш вызывающую функцию не возвращается, коверкая и ломая все ваши код.

    Отвязать событие через таймер событий setTimeout(()=>ticTacToe.win(ticTacToe.player),0); или использовать сообщение API (см. переписать для более)


  • TicTacToe должны иметь только один экземпляр, так что не сделать его instantiatable объекта.

  • Защиты вашего состояния. В ticTactoe объект должен предоставлять только то, что нужно, чтобы использовать его.

    Добавление свойств к, что вы веть не означает, что вы не можете доверять государству объекта. Код ошибки, который ждет

    Использовать замок для защиты состояние объекта.


  • Одну строку функции, когда-то используемые в Кодексе, просто шум и должны быть inline.

  • Быть эффективным. Вы разделили многие функциональные действия, и каждый раз, когда вы звоните в различных частях вашего пересчитывать одни и те же данные.

    Вы создаете таблицу игровых ячеек, то в запросе дом, чтобы получить каждый элемент, а затем пересчитать индекс ячейки, чтобы создать ячейку. Путем добавления двух строк кода в функции создания таблицы вы можете устранить многие другие линии (см. Автор)

    Для выигрышных ходов, вы перебираете ячейки индексы 3 раза, когда вам нужно только сделать это один раз.


  • Вам не нужно бросать ошибки, которые содержат документацию. Я не вижу хорошая причина, чтобы бросать любые ошибки, как это должно быть кода и ошибок быть не должно!.

Рерайт

Пример как переписать, не так, как я написал его.

Я изменить интерфейс немного.

А не создать new TicTacToe(players, size) вы инит существующий экземпляр через ticTacToe.init(players, size) и это возвращает таблицу.

Добавление событий через ticTacToe.addEvent(name, listener). События отделены от кода, так что внешние ошибки не мешают вашим государственного управления. Они также всегда будут стрелять в правильном порядке.

const TicTacToe = (function () {
var resolved, winningCombos, players, cells, table, selCellCount, ready, messageId, eventQueue;
const events = {turn : null, win : null, draw : null, select : null};

/* Fast events will call callback as soon as call stack is clear rather than delayed via setTimeout */
function fireEvent(name, ...args) {
if (events[name]) {
if (messageId === undefined) {
messageId = Date.now() + Math.random() * (2<<30); // random key to stop 3rd party code
eventQueue = [];
addEventListener("message", eventHandler, true)
}
postMessage(messageId, "");
eventQueue.push({name, args});
}
}
function eventHandler(event) {
if (event.source === window && event.data === messageId) {
const call = eventQueue.shift();
events[call.name](...call.args);
event.stopPropagation();
}
}

function createWinCombos(size) {
winningCombos = [];
const diag = [];
const diag1 = [];
combos.push(diag);
combos.push(diag1);
for (let i = 0; i < size; i++) {
const hor = [];
const vert = [];
combos.push(hor);
combos.push(vert);
for (let a = 0; a < size; a++) {
hor.push(a + (i * size));
vert.push(i + (a * size));
if (i === a) { diag.push(i + a * size) }
if (i === (size - 1) - a) { diag.push(i + a * size) }
}
}
return combos;
}

function createTable(size) {
cells = [];
selCellCount = 0;
const table = document.createElement('table');
for (let i = 0; i < size; i++) {
const tr = table.appendChild(document.createElement('tr'));
for (let a = 0; a < size; a++) {
const cell = tr.appendChild(document.createElement('td'));
cell.dataset.num = size * i + a;
cells.push(new Cell(cell));
}
}
return table;
};

function Cell(element) {
var player;
const cellIdx = Number(element.dataset.num);
const cell = {
get value() { return player },
set value(who) {
if(who === ""){
if (player) {
element.textContent = '';
if (player.className) {
element.classList.remove(player.className);
}
player = undefined;
selCellCount --;
}
}else{
if (!player) {
player = who;
player.moves.push(cellIdx);
element.textContent = player.char;
if (player.className) {
element.classList.add(player.className);
}
fireEvent("select", element, player);
selCellCount ++;
}
}
},
}
return cell;
}

function resolve() {
const winner = players.find(player =>
winningCombos.some(combo =>
combo.every(num => player.moves.includes(num))
)
);
if (winner) { fireEvent("win", winner) }
else if (selCellCount === cells.length) { fireEvent("draw") }
else { return false }
return resolved = true;
}

const API = {
addEvent(name, listener) {
if (typeof listener === "function" && events[name]) { event[name] = listener }
},
removeEvent(name) {
if (events[name]) { events[name] = undefined }
},
init(thePlayers, size = 3) {
players = thePlayers.map(player => ({...player, moves : []}));
createWinCombos(size);
table = createTable(size);
resolved = false;
ready = true;
return table;
},
set player(who) {
if (ready) {
for(var i = 0; i < players.length; i++) {
if(players[i].char === who.char){
players.unshift(players.splice(i, 1)[0]);
if(! resolved) { fireEvent("turn", players[0]) }
return;
}
}
}
},
reset(startPlayer = ready ? players[0] : null) {
if (ready) {
cells.forEach(cell => cell.value = "");
API.player = startPlayer;
resolved = false;
}
},
selectCell(element) {
if (ready && ! resolved) {
const cell = cells[element.dataset.num];
if (cell){
cell.value = players[0];
if(cell.value === players[0]){
resolve()
API.player = players[1];
}
}
}
},
};
return API;
})();

Обновление

Ответы на некоторые ваши комментарии


пункт #1 я думаю, что объявления функций -- при использовании Варс -- не рекомендуется (подъем сложностей). Но на самом деле я не знаю, если это дело с пусть и переменных const, и я предполагаю из вашего ответа, что это не так. Не consting все функции, хоть и хорошая вещь ?

Если вы хотите с помощью выражения функции, то вы их.


пункт #2 Итак, я должен использовать переменные closured вместо _xyz-как переменные ?

Закрытие является очень мощным, и вы должны использовать его так много и часто, как вы можете получить опыт. Застежка очень надежная, быстрая, и не переносить синтаксических багажа


Какие другие приложения подчеркивания-предшествующий-переменная стиле, если это плохо, то почему она общая ?

Подчеркивания используются такие языки, как C/C++, чтобы убедиться, что все переменные и функции в библиотеках не конфликтовал, однако это только конвенции не являются частью языка. Чаще всего это будет двойным подчеркиванием __Name

Это также используется в других языках по той же причине, что чаще всего в качестве одного символа подчеркивания.

В JavaScript это было принято как знак того, что переменная является частным, и особенно распространен в код, который использует class знак, который не допускает частной собственности.

Это плохо, потому что язык не соблюдать правила. _private это доверие не исполняются на языке.

Вы можете использовать его, если вам нравится, но вы все равно должны относиться к нему как общественная собственность, если вы хотите, чтобы доверять государству объекта. И если вы идете через усилие обеспечить сохранность имущества, то зачем заморачиваться с подчеркнуть в первую очередь.


Я должен добавить символ подчеркивания перед каждой функцией closured, что не видно снаружи ?
"и не сделать хороший код" не могли бы вы остановиться на этом ?

Нет, вы не должны подчеркнуть. Подчеркивания не имеет смысла, и, таким образом, делает для плохой имя переменной. Это просто шум, что делает код труднее читать.


пункт #7 разве это не похожие на точку № 2 ? И я думаю, вы говорите о обработчики, верно ?

Как вы выставляете свойства объекта до вас.

В следующие значения num1, num2 не являются критическими для поддержания государства. Они могут иметь любое значение и ваш код будет работать нормально.

myObj = {
num1 : 0,
num2 : 0,
test(){
if(myObj.num1 !== myObj.num2){
log("Not the same");
}
}
}

в следующем примере значения num1и num2 могут влиять на состояние кода.

var numberOfSucessfullTests = 0;
const myObj = {
num1 : 0,
num2 : 0,
test(){
myObj.num1 + myObj.num2;
numberOfSucessfullTests += 1;
},
get testCount() { return numberOfSucessfullTests }
}
// which can break with
myObj.num1 = {toString(){throw "error"}};
myObj.num2 = 1;
myObj.test(); // will throw

...
log(myObj.testCount); // 0 state is corrupted

Вы protext выступает государство путем проверки выставленного значения

var numberOfSucessfullTests = 0;
var num1 = 0;
var num2 = 0;
const myObj = {
get num1() { return num1 },
set num1(val) { num1 = Number(val) },
get num2() { return num2 },
set num2(val) { num2 = Number(val) },
test(){
myObj.num1 + myObj.num2;
numberOfSucessfullTests += 1;
},
get testCount() { return numberOfSucessfullTests }
}
// which can break with
myObj.num1 = {toString(){throw "error"}};
myObj.num2 = 1;
myObj.test();
log(myObj.testCount); // 1 state is correct


пункт #8 а что если я в конечном итоге добавив что-то в мой код, который должен использовать это снова ? Не мне нужно создать функцию, ибо тогда он вообще ? Почему бы мне не сделать это сейчас, если я вижу, что функции могут быть повторно использованы в будущем, учитывая, что они действительно присущи коммунальные услуги (в моем коде) ?

Комментарий код для завершения кода, я не могу Второй думаю, что вы должны сделать с кодом.


пункт #10 я не понимаю. Ошибки я бросаю связаны с использованием API, а не мой внутренний код, поэтому я не думаю, что мое освобождение-код имеет какие-либо ошибки, они происходят только тогда, когда пользователь API передает что-то непригодное к API (но я признаю, что это не в унисон с другими частями API, где я не бросать любые ошибки, даже если API используется неправильно).

Бросая ошибки в моей книге действие последней инстанции, что-то сделать, чтобы продолжать выполнение. Если я брошу, я поймаю. Или вы можете использовать броски, чтобы помочь отладки кода, но который должен быть удален после освобождения.


Вместо того чтобы создавать новые крестики-нолики(игроков, размер) вы существующий экземпляр инициализации через крестики-нолики.инит(игроков, размер) и возвращает таблицу. Это, как я хотел, чтобы это было, но я не мог так думать, я мог бы это и новые крестики-нолики(...) конструктор в то же время, я хотел использовать объекты и прототипы.

Прототипы оснащены плюсы и минусы.


  • Плюсы. Они используют меньше памяти в экземпляр объекта, и чуть быстрее, чтобы инстанцировать (обратите внимание, это зависит от браузера, на функции ФФ экземпляра не улучшается при использовании прототипа).

  • Минусы. Прототипы очередной шаг на ссылку/дерево наследования и, таким образом, каждый раз, когда вы используете прототип собственность есть небольшое процессора казни, так как она сначала ищет объект для функции, и если не находит, то он ищет прототип

Прототипы использовали, чтобы быть хорошим для объектов, которые часто создаются или силы многих случаях. Сейчас память дешевая, ГК намного лучше, что делает затраты на прототипы не стоит и пользы вы не получите обработке и использовании объектов бассейна вы можете избежать создания экземпляра и накладных расходов памяти почти полностью.

Есть только один использовать для прототипов, и это прототипное наследование.


Я отделил выигрышные комбинации создавать функции, потому что я думал, что они будут казаться менее сложной, когда разбивается на разные функции ?
Вы думаете, что нельзя просто волнует этот микро-вещи ?
Это компромисс между первоначальной трудоемкость и сложность/неэффективности добавлены путем создания нескольких функций для каждого шага просто не стоит ?

Степень зернистости-номер функции в исходный код. Более детализированные код является больше функций.

Низкая детализация (не многие функции) - это плохо по многим причинам (я предполагаю, что это самоочевидные) так ломать общие задачи для функций выгодно. Но есть точка, где повышение гранулярности код начинает влияние источника качество и выполнение кода эффективность кода.

В исполнении оптимизатор тратит много времени на работу, какие функции не нужны, и в строках их, это потому, что вызовы функций требуют от процессора и памяти выше, что то, что делает функция. Вызов функции имеет свою цену, и вы можете не полагаться на оптимизатор рядный их для вас.

При чтении кода впервые (или возвращаясь к код через какое-то время) функции может нарушить понимание кода, потому что вы должны следовать вызов функции к исходной позиции. Множество функций делает чтение кода сложнее (спагетти вызовов функций).

Находя баланс приходит с опытом. Один вкладыши использован один раз-это плохо.

2
ответ дан 25 февраля 2018 в 04:02 Источник Поделиться