Skip to content

Щукина Ирина#22

Open
iriskis wants to merge 1 commit intourfu-2016:gh-pagesfrom
iriskis:gh-pages
Open

Щукина Ирина#22
iriskis wants to merge 1 commit intourfu-2016:gh-pagesfrom
iriskis:gh-pages

Conversation

@iriskis
Copy link
Copy Markdown

@iriskis iriskis commented May 10, 2017

Copy link
Copy Markdown

@mokhov mokhov left a comment

Choose a reason for hiding this comment

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

Не хватает тестов на игру, т.е на последовательность кликов, что они отрабатываются корректно. Сейчас все тесты на один клик

Comment thread index.js
}
}
if (cellId + 1 < 16 && cellId % n !== n-1) {
if (parseInt(cells[cellId + 1].value) === -1){
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему не объединила в одном if?

Comment thread index.js
}
}

if (board === undefined){
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if (!board)

Comment thread index.js
}
buttons[i][j] = button;
playGround.appendChild(button);
if ((i*4 + j + 1) % n === 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CodeStyle немного страдает

Comment thread index.js
if (playGround.addEventListener) {
playGround.addEventListener('click', swap);
} else {
playGround.attachEvent('onclick', swap);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread tests/index-test.js
})).to.have.length(15);
});

it('should move the cell to a neighboring empty cell', function() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Здесь хорошо бы 4 теста, т.к бывают клетки снизу, сверху, справа и слева.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants