1
\$\begingroup\$

I'm making use of RxJS and observables within my Angular application to process the results of API calls and process data. I've written the component and it works without any issues. However after further research into RxJS and observables I've became aware that the way in which I am using subscriptions is considered bad practice.

Essentially I have a function that subscribes to an observable, then inside the subscription a second function is called which doesn't make use of the data from the first subscription, but subscribes to a separate observable to track the state of a particular variable. Then within the same function a third method is called which does make use of data passed from the first subscription and uses it to make an API call and return some data.

What I want to do is refactor this code so that the same behaviour occurs but so that I am making use of switchMap or other RxJS functions so that the bad practice within my code is removed. However I am unsure as to how I should be employing the use of switchMap etc.

I'll post my code below with each function I have described and annotated.

initSettings(){
    this.getOrganisations$().subscribe( (res: AdminOrganisation[]) => {  //This is the first subscription that makes a get request to return an array of AdminOrganisation objects
      this.organisations = res;                                          //this results of this subscription are used to display different input options to the user in the form of
      this.serial = this.organisations[0].serialRef;                     //a mat-list.
      this.currentOrganisationName = this.organisations[0].serialName;
      this.buildSettingsForm(this.serial);
    });
  }

  buildSettingsForm(serial: string) {
    this.ipRangeFormGroup = this.formBuilder.group(
        {
          ipRanges: this.formBuilder.array([])
        }
    );

    this.numArrayElements = this.ipRangeFormArray.length;

    this.ipRangeFormArray.valueChanges.subscribe(changes => {           //This is the second subscription, this one does not require any external inputs but does rely on the 
      this.numArrayElements = this.ipRangeFormArray.length;             //this.ipRangeFormArray being instantiated before it can be called, therefore it has to be create 
    });                                                                 //syncronously and after the first subscription has recieved and utilised data.

    this.setSettings(serial);
  }

  setSettings(serial: string) {                                                     //This is the third subscription, this subscription utilises the this.serial variable that
    this.getSettingsFromSerial$(serial).subscribe(val => {                          //is retrieved by the first subscription and therefore relies on the data from the first
      this.savedSettingsState = val;                                                //observable to function. Like the second sub, this one also needs to occur after the first
      this.ipRestrictionEnabled = val.ipRestrictionSettings.ipRestrictionEnabled;   //has processed its data.
      for (const i of val.ipRestrictionSettings.ipRanges) {
        this.addRange(i.startRange, i.endRange, i.label);
      }
      this.displayForm = true;
    });
  }

Once I have a grasp on how I should be utilising switchMap/mergeMap/etc I'll be more confident in making this refactorings and improvements myself. But since I am relatively new to Angular I'm unsure as to what the best practice is when making use of these functions to prevent chains of subscriptions like what I have below.

\$\endgroup\$
  • \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ – BCdotWEB Jul 26 at 13:40
  • \$\begingroup\$ Please tell us what this code accomplishes, and show enough context so that we can understand it. See How to Ask. Also, asking for a specific rewrite is tantamount to asking for code to be written, which is not really in the spirit of Code Review. \$\endgroup\$ – 200_success Jul 26 at 14:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.