heyroach / gameoflife Goto Github PK

Сейчас, во-первых, нестандартные имена. life играет роль модели, лучше так и назвать файл и модуль как model. Вместо controller у тебя сейчас ui. Внутри моделей есть обращение во вью и даже вызов view.render(lifeCells) - это все мешанина. Контроллер должен быть связующим звеном, а модель и вью про друг друга в идеале вообще знать не должны :)

Так как в 2016ом году ES6 является де факто стандартом в веб разработке, обязательно нужно перевести проект на ES6. Вот краткое ревью ЕS6 http://es6-features.org/#Constants .

В файле model.js вполне возможно (и даже нужно) отрефакторить огромное if условие с использованием switch'а и, возможно, тернарных операторов. Вот одна из возможных стратегий https://goo.gl/WvKzq8

Ещё обязательно нужно поставить ESLint ( http://eslint.org/ ) и настроить простой галп таск который будет проверять код на ошибки стиля. Мы пользуется стайлгайдом от AirBnB, соответственно файл .eslintrc нужно настроить должным образом.

File: assets/js/src/model.js
Line: 13
Snippet: $.each(model.matrix, function(index, matrix) {

To do:

1. Replace model.matrix with this.matrix, it should work too, but will remove dependency from variable from closure;
2. Replace function instruction with arrow function.
3. Instead of $.each you may be want to use native iterator function matrix.forEach.

А у тебя сейчас это меняется в контроллере. Не надо контроллеру лезти в DOM

Туда вынести описание проекта, зависимости, две команды, чтоб можно было команды оставить
npm run build

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

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

Кристина, осталось только встретиться и обсудить успехи обучения, посмотреть, что из теории улеглось в памяти и обсудить работу :)

cells[iy + matrix[0]] === undefined || cells[iy + matrix[0]][ix + matrix[1]] === undefined не надо
а надо !cells[iy + matrix[0]] || !cells[iy + matrix[0]][ix + matrix[1]]

Сейчас у тебя состояние, жива клетка или нет, хранится в клетке таблицы - это плохо, так как view должно только отображать. На него нельзя полагаться, как на достойный источник информации :)
Поэтому в идеале проверку, жива клетка или нет, надо делать в модели (вместо if ($(this).hasClass('dead'))). Так же надо менять свойства клетки именно в модели, а вью просто перерендерить в соответсвии с новым состоянием модели (это я про текущую строку view.setState(y, x, 'alive');)

Сейчас ты их сохраняешь в id у элемента, разделив решеткой. Лучше используй атрибуты, наподобие data-ordinate и data-abscissa.

Если проект создан на jQuery и активно его использует - лучше этому следовать везде :)
Это помогает код более читаемым сделать, как минимум, так как
var row = document.createElement("tr");
менее читаемый вариант, чем
var row = $("<tr></tr>");
и соответственно в view.table положить jQuery-обернутые объект:
table: document.getElementById("view"),
заменить на
table: $("#view"),

То есть model.timer = ... внутри контроллера - не есть хорошо :)