Skip to content

js_task_2 task_1 task_2#20

Open
MikhailBobrov wants to merge 5 commits into
RTF-Angular-2021:masterfrom
MikhailBobrov:bobrov_mikhail
Open

js_task_2 task_1 task_2#20
MikhailBobrov wants to merge 5 commits into
RTF-Angular-2021:masterfrom
MikhailBobrov:bobrov_mikhail

Conversation

@MikhailBobrov
Copy link
Copy Markdown

Comment thread src/task_1/index.js Outdated
const typeOfPhone_2 = new RegExp("^\\+7\\d{10}$");
const typeOfEmail = new RegExp("^\\w+@\\w+\\.\\w+$");

let item = {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

item нигде не переприсвается, лучше в таких случаях использовать const вместо let

Comment thread src/task_1/index.js Outdated
@returns {boolean} Результат добавления
*/
function add(phoneBook, phone, name, email) {
const typeOfPhone_1 = new RegExp("^\\+7\\-\\d{3}\\-\\d{3}\\-\\d{2}\\-\\d{2}$");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Следует придерживаться одного стиля именования переменных, для js это предпочтительно camelCase, а у тебя тут смешанный camelCase и snake_case.

Comment thread src/task_1/index.js Outdated
*/
function add(phoneBook, phone, name, email) {
const typeOfPhone_1 = new RegExp("^\\+7\\-\\d{3}\\-\\d{3}\\-\\d{2}\\-\\d{2}$");
const typeOfPhone_2 = new RegExp("^\\+7\\d{10}$");
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 src/task_1/index.js Outdated

let item = {}

if (phone.match(typeOfPhone_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.

метод match возвращает сопоставленное значение, ты его нигде не используешь, тут будет уместнее использовать метод test() у регулярки, который возвращает bool

Comment thread src/task_1/index.js Outdated
if (phone.match(typeOfPhone_1)) {
item.phone = phone;
}
else if(phone.match(typeOfPhone_2)) {
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 src/task_3/index.js Outdated

function find(phoneBook, query) {
if (query === '*') {
let firstAr = [];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

const. Почему бы не сделать один общий массив результатов, и возвращать его, нежели каждый раз плодить новый

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ок, сделал

Comment thread src/task_3/index.js Outdated

let oldFormatPhone = phoneBook[key].phone; // +7-922-555-35-35

let newOne = oldFormatPhone.replace(/(\+\d)(\-?)(\d{3})(\-?)(\d{3})(\-?)(\d{2})(\-?)(\d{2})/,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Вынести повторяющуюся функциональности в 3х ifах в отдельную функицю

Comment thread src/task_3/index.js Outdated
}

}
return thirdAr
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 src/task_4/index.js Outdated
return array_1.length;
}

else if (query === 'andrey') {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Используй написанный метод find в предыдущем задании, а здесь напиши только удаление

Comment thread src/task_5/index.js Outdated
if (item.match(typeOfPhone_1)) {
objItem.phone = item;
}
else if (item.match(typeOfPhone_2)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Комменты из 1го задания актуальны

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.

3 participants