Skip to content

Бёрдов Дмитрий#36

Open
CLearERR wants to merge 8 commits into
urfu-2016:masterfrom
CLearERR:master
Open

Бёрдов Дмитрий#36
CLearERR wants to merge 8 commits into
urfu-2016:masterfrom
CLearERR:master

Conversation

@CLearERR

Copy link
Copy Markdown

No description provided.

@Zhigalov Zhigalov self-assigned this Apr 27, 2017
@Zhigalov

Copy link
Copy Markdown
Contributor

Код не собирается
image

@CLearERR

CLearERR commented May 1, 2017

Copy link
Copy Markdown
Author

Файл компилируется, расширено покрытие тестами.

@Zhigalov Zhigalov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Переделай работу с ошибками, после этого посмотрю ещё раз

Comment thread getPokerHand.js Outdated
return 'Покер';
try {

dice.sort();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Проверить является ли переменная массивом можно оператором Array.isArray(dice)

Comment thread getPokerHand.js Outdated
catch (err) {

res = ('Не массив');
return res;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ошибку надо не возвращать return а выбрасывать throw:

throw new Error('Не массив');

});


describe('getPokerHand', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Нужно объявить один describe а внутри него несколько it:

describe('getPokerHand', () => {
   it('should ...', () => {});
   it('should ...', () => {});
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Это замечание всё ещё актуально

@CLearERR

CLearERR commented May 2, 2017

Copy link
Copy Markdown
Author

Переделана работа с ошибками, из TODO - рефакторинг файла с тестами.

@Zhigalov Zhigalov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Уже гораздо лучше!

Comment thread getPokerHand.js

j = 0;
for(var i = 1; i<5; i++){
if ((isNaN(dice[i])) || (isNaN(dice[j]))) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

А можно ограничиться только одной проверкой?
То есть не два числа проверять, а только одно во всех случаях?

});


describe('getPokerHand', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Это замечание всё ещё актуально

} catch (error) {
assert.equal(error.message, 'Не массив');
}
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Попробуй воспользоваться assert.throws
Твои тесты станут компактнее
Документацию можно почитать по ссылке https://nodejs.org/api/assert.html#assert_assert_throws_block_error_message

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