am I using anti-patterns in this implementation?

59 views Asked by At

what i want to to in this component is straight-forward getting data of a selected question by the route id param. difference is that i'm trying to make things as declarative as possible so my component looks like this:


export class QuestionDetailsComponent{
  constructor(
    private questionService: QuestionsService,
    private router: Router,
    private route: ActivatedRoute,
    private answersService: AnswersService,
    private authService: AuthService
  ) {}
  readonly activeUser$ = this.authService.activeUserOf;

  questionId$ = this.route.paramMap.pipe(
    map((paramMap) => paramMap.get('id')),
    filter(Boolean)
  );

  question$ = this.questionId$.pipe(
    switchMap((questionId$ => this.questionService.getActiveQuestion(questionId$)))
  )

  answers$ = this.questionId$.pipe(
    switchMap(questionId => this.answersService.getByQuestionId(questionId))
  )

  deleteId$ = new Subject<number>();

  deleteStream$ = this.deleteId$.pipe(
    switchMap((deleteId) =>
      this.questionService.deleteQuestion(deleteId).pipe(
        tap(() => this.router.navigate([''])),
        catchError((error) => {
          console.log('question has not been deleted');
          return throwError(() => error);
        })
      )
    )
  );

  onDelete(deleteId: number) {
    this.deleteId$.next(deleteId);
  }
}

i'm not sure about this solution, is there a better way to do this type of thing? is not using ngOnInit a good solution in general?

1

There are 1 answers

0
Andrei On

This question sounds opinionated. Personally I don't see any bad reasons to write components as in the question and I don't see any problems apart from - the should be subscription to deleteStream$ for it to work, and there is no really pretty place to put this subscription somewhere in the component.

a few improvements I would propose:

use of vm$ to make multiple usages of async pipe as one

vm$ = combineLatest({question: this.question$, somethingElse: this.somethingElse$})

// template
<ng-container *ngIf="vm$ | async as vm">
  {{ vm.question.id }} // every value is accessible anywhere in the template without async pipe

and I really like using ngrx (with or without components store) with its effects. so the deletion stream (and any other async stuff) would look prettier

deleteStream$ = createEffect(() => this.actions$.pipe(ofType(deleteQuestion), ....), {dispatch: false})