Repeated code using filter rxjs - angular, how to improve it?

Viewed 74

In my appComponent I have quite a lot of logic that needs to get executed each time the application starts. The problem is that there are some common observables and then others that sometimes need to be executed depending on some conditions. I use filter to establish those conditions and to allow a chain of observables to continue. In the end, I end up repeating a lot of parts of the code.

This occurs to me in other parts of the application too. Is there a better way to do this? I think I am repeating a lot of code and it has no sense.

If you check the code, I think you will understand better. Check that I use filter to decide to proceed or not.

const sessionVariables$ = this.userService.getUserObservable().
  pipe(
  concatMap((response: UserImi) => {
    this.user = response;
    return this.sessionVariablesService.getUserSesionVariablesByRegistrationNumber(this.user.user)
  })

//Si no existe sesion abrimos el modal de ramas para seleccionar la oficina, o  se escoge la oficina directamente si no tienes grupo funcional
sessionVariables$
  .pipe(
    filter((response: SessionVariables) => !response
  ).subscribe((response: SessionVariables) => {
    this.sessionVariablesService.actualSessionVariables = response;
    this.openModalBranch();
  })

//Si existe sesion y la oficina es de qmatic  miramos si existe atención para cargarla y otras cosas y despues obtenemos el userStatus
sessionVariables$
  .pipe(
    filter((response: SessionVariables) => !!response && response.branch.queues == 'Y'),
    tap((response: SessionVariables) => {
      this.sessionVariablesService.actualSessionVariables = response;
      this.branchService.branchLogged = this.sessionVariablesService.actualSessionVariables.branch;
  })
  .pipe(
    concatMap(() => {
      return this.notificationInformerBranchService.getNotificationInformerBranchByInformerAndBranch(this.user.user, this.branchService.branchLogged.branchId.toString())
    })
  )

//Si existe sesion y la oficina no es de Qmatic
sessionVariables$
  .pipe(
    filter(
      (response: SessionVariables) =>
        !!response && response.branch.queues == "N"
    ),
    tap((response: SessionVariables) => {          
      this.sessionVariablesService.actualSessionVariables = response;
      this.branchService.branchLogged =
        this.sessionVariablesService.actualSessionVariables.branch;
      //Vamos a la home de las oficnias sin qmatic
        this.router.navigateByUrl("/nocues/home");
    }).pipe(
    concatMap(() => {          
      return this.notificationInformerBranchService.getNotificationInformerBranchByInformerAndBranch(this.user.user, this.branchService.branchLogged.branchId)
      })
    )
1 Answers

If I understand your logic right, I would refactor your code this way. Comments are inline

// start defining the basic Observable chain, i.e. the chain that you want always to execute, no matter which are the cases
const sessionVariables$ = this.userService.getUserObservable().pipe(
  concatMap((response: UserImi) => {
    // place the following line here, since it looks like you want always to run it
    this.sessionVariablesService.actualSessionVariables = response;
    this.user = response;
    return this.sessionVariablesService.getUserSesionVariablesByRegistrationNumber(this.user.user)
  })
);

// then define an Observable for each case and store it in separate variables
// Si no existe sesion abrimos el modal de ramas para seleccionar la oficina, o  se escoge la oficina directamente si no tienes grupo funcional
const noSession$ = sessionVariables$.pipe(
  filter((response: SessionVariables) => !response),
  // do not subscribe here, but inject the logic into a tap operator
  tap((response: SessionVariables) => {
    this.openModalBranch();
  })
);


// Si existe sesion y la oficina es de qmatic  miramos si existe atención para cargarla y otras cosas y despues obtenemos el userStatus
const gmatic$ = sessionVariables$.pipe(
  filter((response: SessionVariables) => !!response && response.branch.queues == 'Y'),
  tap((response: SessionVariables) => {
    this.branchService.branchLogged = this.sessionVariablesService.actualSessionVariables.branch;
  }),
  concatMap(() => {
    return this.notificationInformerBranchService.getNotificationInformerBranchByInformerAndBranch(this.user.user, this.branchService.branchLogged.branchId.toString())
  })
);


// Si existe sesion y la oficina no es de Qmatic
const noGmatic$ = sessionVariables$.pipe(
  filter((response: SessionVariables) => !!response && response.branch.queues == "N"),
  tap((response: SessionVariables) => {
      this.branchService.branchLogged = this.sessionVariablesService.actualSessionVariables.branch;
      // Vamos a la home de las oficnias sin qmatic
      this.router.navigateByUrl("/nocues/home");
  }),
  // not sure what getNotificationInformerBranchByInformerAndBranch does and why you want it here
  // considering that you are navigating to another page via router
  concatMap(() => 
    return this.notificationInformerBranchService.getNotificationInformerBranchByInformerAndBranch(this.user.user, this.branchService.branchLogged.branchId)
  })
);

// eventually subscribe to all the above Observables once using merge
merge(noSession$, gmatic$, noGmatic$)
.subscribe()

If your services are basically doing http calls, then you do not need to worry about unsubscribing. Otherwise you may have to add some logic to make sure you unsubscribe when the component is destroyed, maybe using the ngOnDestroy() lifecycle method or other equivalent techniques.

Related