Skip to content

Conversation

@sofelvpopel
Copy link

No description provided.

@honest-hrundel honest-hrundel changed the title qwe Гафурова Софья Oct 24, 2018
@honest-hrundel
Copy link

🍅 Пройдено тестов 21 из 26

@honest-hrundel
Copy link

🍅 Пройдено тестов 21 из 23

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройдено тестов 23 из 23

phone-book.js Outdated
* Реализован метод importFromCsv
*/
const isStar = true;
const isStar = false;
Copy link

@Xrias Xrias Oct 25, 2018

Choose a reason for hiding this comment

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

У тебя код написан на звездочку, а здесь false?

*/
function add(phone, name, email) {
function add(phone, name = '', email = '') {
const validPhone = /^(\d)\1\1(\d)\2\2(\d)\3(\d)\4$/.test(phone);
Copy link

Choose a reason for hiding this comment

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

Можно же /^\d{10}$/.test(phone). Повтори регулярки.

const validPhone = /^(\d)\1\1(\d)\2\2(\d)\3(\d)\4$/.test(phone);
const contact = phoneBook[phone];

if (!validPhone || !name || contact) {
Copy link

Choose a reason for hiding this comment

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

!validPhone || !name || phoneBook[phone]
Можно и не выносить в переменную

const result = findAll(query);

return result.map(c => {
let r = `${c.name}, ${toFullPhone(c.phone)}`;
Copy link

Choose a reason for hiding this comment

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

Переименуй r и c на что-то информативное.

return allContacts;
}

function toFullPhone(phone) {
Copy link

Choose a reason for hiding this comment

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

formatPhone?

@honest-hrundel
Copy link

🍅 Пройдено тестов 23 из 26

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants