Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial review #1

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.idea
14 changes: 13 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,13 @@
# js-minesweeper
# js-minesweeper

## Основные правила:

* Для начала игры необходимо указать размер игрового поля и количество мин и нажать "Start";
* Открытие ячейки осуществляется кликом левой кнопкой мыши;
* Установка/снятие флажка - правая кнопка мыши;
* Игра заканчивает в случае, если:
* Все ячейки с минами помечены фалжками, а все остальные ячейки открыты - победа;
* Открыта любая ячейка с миной - поражение.
* В случае победы результат (количество потраченных на игру секунд) фиксируется локалльно (localStorage), выводится таблица с результатами.
Текущий результат выделен серым цветом.

Binary file added assets/flag.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/mine.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
105 changes: 105 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>MineSweeper</title>
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.3.0/lodash.js"></script>
<script defer src="js/constants.js"></script>
<script defer src="js/dispatcher.js"></script>
<script defer src="js/timer.js"></script>
<script defer src="js/controls.js"></script>
<script defer src="js/fieldModel.js"></script>
<script defer src="js/gameModel.js"></script>
<script defer src="js/results.js"></script>
<script defer src="js/modal.js"></script>
<script defer src="js/game.js"></script>
<script defer src="js/app.js"></script>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

отличная идея ставить defer в начале скрипта!
так гораздо лучше читается

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

<link rel="stylesheet" href="style/style.css">
</head>
<body>
<div class="wrapper">
<div class="game-wrapper" data-component="game">
<div class="controls-wrapper" data-component="controls">
<!-- Controls will be rendered here -->
</div>
<div class="field-wrapper" data-component="field">
<!-- Game field will be rendered here -->
</div>
<!-- Modal will be rendered here -->
</div>
</div>
<!-- Templates -->
<script type="'text/template" id="controls-template">
<div class="controls-col">
<form name="options-form">
<label for="rows" class="controls-element">
<span class="control-name">Rows (<%- options.minRows %>-<%- options.maxRows %>)</span>
<input type="number" name="rows" id="rows" class="control-input" value="<%- options.rows %>" min="<%- options.minRows %>" max="<%- options.maxRows %>">
</label>
<label for="columns" class="controls-element">
<span class="control-name">Columns (<%- options.minCols %>-<%- options.maxCols %>)</span>
<input type="number" name="columns" id="columns" class="control-input" value="<%- options.cols %>" min="<%- options.minCols %>" max="<%- options.maxCols %>">
</label>
<label for="mines" class="controls-element">
<span class="control-name">Mines <span data-range = "mines">(<%- options.minMines %>-<%- options.maxMines %>)</span></span>
<input type="number" name="mines" id="mines" class="control-input" value="<%- options.mines %>" min="<%- options.minMines %>" max="<%-options.maxMines %>">
</label>
</form>
</div>
<div class="controls-col controls-col-right">
<span class="timer" data-content="timer">00:00</span>
</div>
<div class="start-btn" data-action="start">Start</div>
</script>
<script type="text/template" id="field-template">
<table class="field">
<% for (let i = 0; i < rows; i++) { %>
<tr>
<% for (let j = 0; j < columns; j++) { %>
<td class="cell"></td>
<% } %>
</tr>
<% } %>
</table>
</script>
<script type="text/template" id="results-template">
<div class="fixed-container">
<div class="fixed-container-inner">
<table class="table-fixed">
<thead>
<tr>
<th>
<div class="th-fixed-inner">Rank</div>
</th>
<th>
<div class="th-fixed-inner">Result (sec)</div>
</th>
<th>
<div class="th-fixed-inner">Date</div>
</th>
</tr>
</thead>
<tbody>
<% results.forEach(function(result, i) { %>
<tr <% if(result.current) { %>class="current"<% } %>>
<td><%= i+1 %></td>
<td><%= result.time %></td>
<td><%= result.date %></td>
</tr>
<% }); %>
</tbody>
</table>
</div>
</div>
</script>
<script type="text/template" id="modal-template">
<div class="modal modal-overlay js-modal-hidden">
<div class="modal-inner">
<h3 class="modal-title"><%- title %></h3>
<div class="modal-content"><%= content %></div>
<div class="modal-close" data-action="close"></div>
</div>
</div>
</script>
</body>
</html>
5 changes: 5 additions & 0 deletions js/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict';

let game = new Game({
element: document.querySelector('[data-component="game"]')
});
24 changes: 24 additions & 0 deletions js/constants.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

const CellType = {
EMPTY: 0,
MINE: -1
};

const GameState = {
START: 0,
WIN: 1,
LOST: 2
};

const Default = {
COLUMNS: 9,
ROWS: 9,
MINES: 10,
MIN_ROWS: 9,
MAX_ROWS: 20,
MIN_COLUMNS: 9,
MAX_COLUMNS: 20,
MIN_MINES: 2,
MAX_MINES: 40
};
175 changes: 175 additions & 0 deletions js/controls.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
'use strict';

var Controls = (function() {
var controlsTemplate = document.getElementById('controls-template').innerHTML;
/**
* Class representing the control panel
*/
class Controls extends Dispatcher {
/**
* Create controls
* @param options
*/
constructor(options) {
super(options);

this._render();
this._timer = options.timer;
this._timerEl = this._el.querySelector('[data-content="timer"]');
this._addListeners();
}

/**
* Render controls from template
* @private
*/
_render() {
let defaultOptions = {
minRows: Default.MIN_ROWS,
maxRows: Default.MAX_ROWS,
minCols: Default.MIN_COLUMNS,
maxCols: Default.MAX_COLUMNS,
minMines: Default.MIN_MINES,
maxMines: Default.MAX_MINES,
rows: Default.ROWS,
cols: Default.COLUMNS,
mines: Default.MINES
};

this._el.innerHTML = _.template(controlsTemplate)({
options: defaultOptions
});
}

/**
* Add main listeners for click (start game), timer update, and options input change
* @private
*/
_addListeners() {
this._el.addEventListener('click', this._onClick.bind(this));
this._el.addEventListener('change', this._onChange.bind(this));
this._timer.addEventListener('timeUpdate', this._onTimerUpdate.bind(this));
}

/**
* 'Click' event handler
* @param {MouseEvent} event - click event
* @private
*/
_onClick(event) {
var startBtn = event.target.closest('[data-action="start"]');

if (!startBtn) {
return
}
this._onStartClick();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

так обычно называют обработчики событий
в данном случае это просто приватный метод

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил на triggerClick, чтобы не было путаницы

}

/**
* Implement start - dispatch 'start' event with current options
* @private
*/
_onStartClick() {
let options = this._getOptions();

this.dispatchEvent('start', options);
}

/**
* 'Change' event hadler
* @param {Event} event - change event
* @private
*/
_onChange(event) {
let optionsForm = event.target.closest('[name="options-form"]');

if (!optionsForm) {
return;
}

this._validateOptions();
}

/**
* Validate current options
* @private
*/
_validateOptions() {
let options = this._getOptions();
//Check if input is not valid number
if (isNaN(options.columns)) {
options.columns = Default.MIN_COLUMNS;
}
if (isNaN(options.rows)) {
options.rows = Default.MIN_ROWS;
}
if (isNaN(options.mines)) {
options.rows = Default.MIN_MINES;
}

options.rows = Math.max(Default.MIN_ROWS, Math.min(options.rows, Default.MAX_ROWS));
options.columns = Math.max(Default.MIN_COLUMNS, Math.min(options.columns, Default.MAX_COLUMNS));

//Calculate mines max range
let maxMines = this._getMinesMaxRange(options.rows, options.columns);
options.mines = Math.max(Default.MIN_MINES, Math.min(options.mines, maxMines));
//Set mines range
this._el.querySelector('[data-range="mines"]').textContent = `(${Default.MIN_MINES}-${maxMines})`;
this._el.querySelector('[name="mines"]').max = Default.MIN_MINES;
this._el.querySelector('[name="mines"]').max = maxMines;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

из предложений по оптимизаци

  1. вынести селекторы в объект
const SELECTORS = {
  minesRange: '[data-range="mines"]',
  ...
};

Чтобы не пришлось искать в коде когда захочется порефакторить

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Можно в конструкторе или при рендеринге сохранить в переменные результаты
    this._el.querySelector('[name="mines"]')
    чтобы не искать их каждый раз заново
    Если конечно поиск каждый раз не даёт новый результат

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил


this._setOptions(options);
}

/**
* Get options from DOM
* @returns {{rows: Number, columns: Number, mines: Number}}
* @private
*/
_getOptions() {
let rows = parseInt(this._el.querySelector('[name="rows"]').value);
let columns = parseInt(this._el.querySelector('[name="columns"]').value);
let mines = parseInt(this._el.querySelector('[name="mines"]').value);

return {
rows: rows,
columns: columns,
mines: mines
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут можно переменные заранее не объявлять
ключи объекта дают ту же информацию что и имена переменных

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убрал повторения

}

/**
* Set options to DOM
* @param options
* @private
*/
_setOptions(options) {
this._el.querySelector('[name="rows"]').value = options.rows;
this._el.querySelector('[name="columns"]').value = options.columns;
this._el.querySelector('[name="mines"]').value = options.mines;
}

_getMinesMaxRange(rows, columns) {
return Math.round(rows*columns/2);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я бы даже тут ставил пробелы :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пропустил. Теперь исправил

}

/**
* Render new timer value
* @param {CustomEvent} event
* @private
*/
_onTimerUpdate(event) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я бы всё таки разделил на 2 метода

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вынес twoDigits в отдельный метод. В каких ситуациях возможно создавать такие вспомогательные функции внутри метода? Или это плохой подход?

let time = event.detail;
let min = Math.floor(time / 60);
let sec = time % 60;

function twoDigits(n) {
return (n < 10 ? '0' : '') + n;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ещё вариант
('0' + n).slice(-2)

}

this._timerEl.textContent = `${twoDigits(min)}:${twoDigits(sec)}`;
}
}

return Controls;
})();
41 changes: 41 additions & 0 deletions js/dispatcher.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use strict';
/**
* Class representing the base event dispathcer
*/
class Dispatcher {
/**
* Crrate dispatcher
* @param options
*/
constructor(options) {
options = options || {};
this._el = options.element || document.createElement('div');
}

/**
* Add event handler
* @param {string} type
* @param {function} handler
* @param {boolean} capture - use capture
*/
addEventListener(type, handler, capture) {
this._el.addEventListener(type, handler, capture);
}

/**
* Dispatch custom event
* @param {string} type
* @param detail - event data
* @param {object} options
*/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Отдельное спасибо за доку!

dispatchEvent(type, detail, options) {
options = options || {};

if (detail != undefined) {
options.detail = detail;
}

let event = new CustomEvent(type, options);
this._el.dispatchEvent(event);
}
}
Loading