search

RYAZHAPOVILNUR / mentoring-users-app

9 stars 82 forks source link

ШАГ 1 {{Литвиненко Роман}} {{@Alucard163}} #258

Closed Alucard163 closed 5 months ago

Alucard163 commented 5 months ago

Переписал джун задание на ngrx https://github.com/Alucard163/typicode-users-angular-app

krymnon69 commented 5 months ago

(click)="userForm.valid ? onDialogCloseClick(userForm.value) : null" лучше просто задизейблить кнопку, либо делать уже в самом методе: if(userForm.invalid) return;

krymnon69 commented 5 months ago

this.userForm = this.formBuilder.group({ лучше проинициализировать сразу, чем писать в конструкторе

krymnon69 commented 5 months ago

this.user?.name || '' вместо этого легче один раз сделать this.userForm.patchValue()

UPD: к тому же ты и так его делаешь, дважды заполняешь форму значениями. но это для patchValue абсолютно лишнее: const modifiedUser = {...this.user};

krymnon69 commented 5 months ago

this.userForm.controls[field] убиваешь типы (обращение через [''] возвращает тип any) для форм придумали userForm.get()

krymnon69 commented 5 months ago

@Output('delete') deleteCard кажется устаревшая и не лучшая практика присваивать другое имя событию (delete)

krymnon69 commented 5 months ago

public usersListFacade = inject(UsersListFacade); public users$ = this.usersListFacade.users$;

это тоже может быть readonly и users$ может вообще не быть, можно обратиться сразу к usersListFacade.users$

krymnon69 commented 5 months ago

data-access относится к архитектуре nx

krymnon69 commented 5 months ago

image

krymnon69 commented 5 months ago

методы в UsersApiService можно назвать без постфикса Users, т.к. они априори относятся к объекту USERSApiService

krymnon69 commented 5 months ago

private localStorageKey = LOCAL_STORAGE_USERS_KEY; private parseJSON = parseJSON; это лишнее (переменные)

krymnon69 commented 5 months ago

shared должен быть один по всему приложению на верхнем уровне (уровне app.comp)

krymnon69 commented 5 months ago

некорректное название файла localStorageHelpers.ts

krymnon69 commented 5 months ago

было бы круто если бы локал сторадж сервис сам занимался парсингом и стрингифаем данных

krymnon69 commented 5 months ago

export const LOCAL_STORAGE_USERS_KEY = 'users'; лучше делать enum:

export enum StorageKey = {
   USERS = 'users'
}
krymnon69 commented 5 months ago

Из-за того что ты делаешь нереальную в реальной жизни задачу - синхронизируешь ngrx с localstorage - много говнокода и DRY с этим связанного. Советую избавиться от синхронизации со storage, учесть замечания но больше для себя. Пишешь не плохо, я считаю стоит поправить для себя, быстро на дейлике пробежимся глазами и пойдёшь дальше. ПРИНЯТО