feat: добавлен вывод массива продуктов и передача выбранного к покупк… #44
Conversation
…е продукта в productList
|
|
||
| @Component({ | ||
| selector: 'app-card', | ||
| templateUrl: './card.component.html', | ||
| styleUrls: ['./card.component.css'], | ||
| }) | ||
| export class CardComponent { | ||
| readonly product = productsMock[0]; | ||
| @Input() product: IProduct | any; |
There was a problem hiding this comment.
Не запускалось без any. Я не силен в TypeScript поэтому поставил чтобы заработало.
There was a problem hiding this comment.
Рекомендую поисследовать, т.к. any, конечно, убирает ошибки, но в бою его использование очень опасно, тем более в таких кейсах - напрочь убиваем типизацию. Скорее всего ошибки летели из-за того, что по дефолту значение undefined, и ты его не указал в типизации. Попробуй поменять any на undefined, это будет корректней и ошибка в этом месте должна полечится, но будь готов, появятся ошибки в других местах, т.к. в значении указан undefined и он действительно может быть в инпуте, по этому стоит обработать этот кейс в других местах
|
|
||
| @Component({ | ||
| selector: 'app-card', | ||
| templateUrl: './card.component.html', | ||
| styleUrls: ['./card.component.css'], | ||
| }) | ||
| export class CardComponent { | ||
| readonly product = productsMock[0]; | ||
| @Input() product: IProduct | any; | ||
| @Output() productBuy: EventEmitter<IProduct> = new EventEmitter<IProduct>(); |
There was a problem hiding this comment.
Не кажется ли тебе, что получается тавтология?)
...: EventEmitter<IProduct> = new EventEmitter<IProduct>();Есть ли в этом сакральный смысл?
Мне кажется, что нет, тут отлично сработала бы неявная типизация свойства, а за счет дженерика можно четко указать тип данных передающихся в Output:
@Output() readonly productBuy = new EventEmitter<IProduct>();Эта запись мне видится лаконичней, без тавтологии, и не менее типизированной, что важно.
Что думаешь по данному поводу?
There was a problem hiding this comment.
Опять повторюсь, что в TypeScript не силен, поэтому возможны всякие тавтологии. Собственно к курсу по angular это не особо касается, верно?
There was a problem hiding this comment.
Да, это не совсем тема Angular, но если мы хотим писать качественный, читаемый код, то хорошие практики в используемых технологиях тоже будут кстати, в частности, чтобы качественно и чисто писать на Angular - нужно знать TS)
Если у тебя есть вопросы касаемо TS, ты можешь их смело задать, либо, могу игнорировать у тебя подобные практики с TS. Выбор за тобой!🙃
|
|
||
| onProductBuy(event: Event) { | ||
| onProductBuy(event: Event, product: IProduct) { |
There was a problem hiding this comment.
Почему решил передавать product в аргументе, а не доставать его через this.product?
There was a problem hiding this comment.
Соблюдаю правило чистой функции.
There was a problem hiding this comment.
Только смотри какой нюанс, у нас тут не чистая функция. Априори то, что мы используем в шаблоне - либо свойство, либо метод класса компонента и я рекомендую этим пользоваться, иначе у нас могут появиться очень перегруженные аргументами функции, что нам явно не упростит жизнь
There was a problem hiding this comment.
В частности, тут бы я использовал this.product, вместо его передачи в аргументе разницы абсолютно никакой для Angular не будет, но для нас - менее загруженный аргументами метод
| @@ -1 +1,3 @@ | |||
| <app-card (click)="onCardClick()"></app-card> | |||
| <div *ngFor="let product of productsMock; index as i; first as isFirst"> | |||
There was a problem hiding this comment.
Бьешь на опередение)
А почему не применил ngFor сразу на <app-card>? Зачем нам лишняя обертка в виде <div>?
There was a problem hiding this comment.
Привычка работы с twig шаблонизатором.
There was a problem hiding this comment.
Как думаешь, есть ли в этом необходимость тут, или только создали лишнюю вложенность элементов?
| onCardClick() { | ||
| productsMock = productsMock; | ||
| onCardClick(product: IProduct) { | ||
| const {name} = product || {}; |
There was a problem hiding this comment.
А зачем тут ||? По типизации нам в product аргументе может придти только IProduct, и ничего больше, а значит product || {} будет всегда возвращать product. Тогда есть ли смысл в данном коде?
There was a problem hiding this comment.
Это также привычка из ванили. Там если такую штуку не прописать, есть вероятность получить typeError.
There was a problem hiding this comment.
Честно, выглядит как костылек, чтобы не получить ошибку при отсутствии product, при чем выглядит это не явно, т.к. мы запрашиваем не существующее свойство у {}. Я бы рекомендовал сделать эту логику более прозрачной
const name = this.product ? this.product.name : undefined;По поведению - то же самое, что и у тебя, но тут явно прослеживается кейс, что при отсутсвии значения - берем undefinde.
There was a problem hiding this comment.
В нашем случае и вовсе можно было бы использовать if и выйти из метода, т.к. без продукта - бесполезно реагировать на его покупку
Что думаешь?
…е продукта в productList