-
Notifications
You must be signed in to change notification settings - Fork 0
справочник #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
справочник #21
Conversation
| ] | ||
| } | ||
| PhoneApp.prototype.phoneNum = function(phoneNum) { | ||
| if (this.testOnNum(phoneNum) && this.ten(phoneNum)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!this._isPhoneNumberValid) {
return `неправильно введен номер`
};
let str = phoneNum;
return `(${str.substring(0,3)}) ${str.substring(3,5)}-${str.substring(5,7)}-${str.substring(7,10)}`;| return arr.every(isNum); | ||
| } | ||
| PhoneApp.prototype.ten = function(phoneNum) { | ||
| return phoneNum.length == 10 || `введите десятизначный номер`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please verify it works as expected
| } | ||
| PhoneApp.prototype.addUser = function(name, phone) { | ||
| let lastId = this.dataBase[this.dataBase.length - 1].id; | ||
| let obj = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
еще один конструктор для пользователя
| this.dataBase.push(obj); | ||
| } | ||
| PhoneApp.prototype.deleteUser = function(name) { | ||
| this.dataBase.forEach((value, i) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use .filter instead of .forEach
| } | ||
| PhoneApp.prototype.search = function(name) { | ||
| let arr = []; | ||
| this.dataBase.forEach((value, i) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please user .filter
| } | ||
| PhoneApp.prototype.filter = function(str) { | ||
| let dataBaseFilter = []; | ||
| this.dataBase.forEach((value, i) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
здесь точно нужно использовать .filter. Правильный выбор метода перестроит этот код верно
это может выглядеть например таким образом
.filter(user => {
let userFields = Object.values(user);
return userFields.some(userData => userData.includes(str))
})Этот пример еще нужно доработать в твоем примере более сложные вещи вот тут:
value[key].length - str.length
value[key].substr(j, str.length) == str
dataBaseFilter.push(value);
return;
Почему этот код мне не нравится, потому что если в дальнейшем его нужно будет дорабатывать, большая вероятность что нужно будет переписать все.
И если ты посмотришь на этот код например даже через месяц, будет сложно понять что тут происходит
| let obj = {}; | ||
| obj.id = lastId + 1; | ||
| obj.name = name; | ||
| obj.phone = this.phoneNum(phone); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если у пользователя будет не верный номер, тогда ему запишется строка неправильно введен номер
И не факт что это будет лучше, я бы сказал что лучше хотя бы null передавать
| }); | ||
| return arr; | ||
| } | ||
| PhoneApp.prototype.change = function(name, newName, newPhone) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
имя метода change мало понятно, change что ?
changeUser ? но это тоже не совсем правда этот метод может что-то изменить, а может и не изменить :)
Тут можно не бояться длинных названий, это будет более информативно. Явное всегда лучше неявного
| return arr; | ||
| } | ||
| PhoneApp.prototype.change = function(name, newName, newPhone) { | ||
| if (newName && newPhone) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вот тут лучше записать
if( !(newName && newPhone) ) {
return
}
this.changePhone(name, newPhone);
this.changeName(name, newName);
No description provided.