Making multiple http request Angular2

Question

I need to get some ids from contacts and contactgroups to be able to collect the data. My example works but i dont know if this is the way to go.

getIdArray() {
let contactIds = [];
let groupIds = [];
let contacts = this.http.get('Contacts/').map(res => res.json());
let contactGroup = this.http.get('ContactGroups/').map(res =>res.json());
Observable.forkJoin([contacts, contactGroup]).subscribe(res => {

  for (let b of res[0].contacts) {
    console.log(b.contactId);
    contactIds.push(b.contactId)
  }

  for (let b of res[1].contactgroups) {
    console.log(b.contactgroupId)
    groupIds.push(b.contactgroupId)
  }
  this.loadContacts(contactIds);
  this.loadGroups(groupIds);

 });
}

loadContacts(contactId) {
console.log("ids" + JSON.stringify(contactId));
Observable.forkJoin(
  contactId.map(
    i => this.http.get('Contacts/' + i)
      .map(res => res.json())
  )
 ).subscribe(data => {
  for (let b of data) {
    console.log(b);
  }
});
}

loadGroups(groupIds){
console.log("ids" + JSON.stringify(groupIds));
Observable.forkJoin(
  groupIds.map(
    i => this.http.get('ContactGroups/' + i)
      .map(res => res.json())
  )
).subscribe(data => {
  for (let b of data) {
    console.log(b);
  }
});
}

Answer 1

Instead Of Preface

My answer is split into three main parts. The first two explain how to make the small-ish code changes to improve readability and decrease fragility. The third part covers more of a design aspect which you may find important.

A. Tactical Improvements

1. TypeScript is about Types

Use explicit types wherever possible. This will:

• Enable compile time errors (very helpful for nasty typo detection);
• Help a lot during the code refactor (especially when it comes to signature changes). However, don't get too confident -- certain things are not being caught by the compiler.

In the code provided below, notice that all the signatures declare parameter types and return type.

1.1. Little Type Hint Trick

Often, frameworks or libraries return objects of type any. When this happens, the compiler loses the narrow type information making the code less reliable. As an example, we can see that response.json() in our Observables' map part is doing exactly that. Since we know, what object the remote service API is supposed to return us back, we can immediately cast that expression to the specific type and make the compiler our helper again: <ContactGroup>response.json().

It's important to understand though, that in runtime there is no guarantee that the return object is actually of the same type as the type we cast to. In other words, this cast is practically helpful, but is sometimes misleading and should be used carefully.

2. Master JavaScript and TypeScript idioms

TypeScript is a superset of JavaScript, i.e. whatever is syntactically correct in JS is also correct in TS.

• Therefore, instead of looping through the items and writing them to console one by one, you can use the spread operator console.log('Contact id list', ...response.contacts), where response.contacts is an array.
• Another thing is the var/let/const. As a rule, I start with const and switch to let only when feel it necessary or pragmatic. This approach forces me to think in a way more functional programming way which I find less error prone compared to imperative style. A thing (variable) does not have to be a constant in physical, mathematical, or business-logic sense of this word like Pi, Epsilon, Gravitational constant, etc. to be declared as const. It's just an instruction to the compiler the you don't want this thing to be reassigned down the road. This practice prevents accidental assignments (source of bugs) and pushes towards cleaner code.

I am very new to JavaScript myself, therefore it is hard for me to notice any other non-idiomatic pieces of code.

B. Slightly Different Code

Please notice that this is not a refactoring since I don't know what your models exactly look like.

    private contacts: Contact[] = [];
    private contactGroups: ContactGroup[] = [];

    loadContactsAndContactGroupsTogether(): void {
      Observable
        .forkJoin([
          this.http.get('Contacts/').map(response => response.json()),
          this.http.get('ContactGroups/').map(response => response.json())
        ])
        .subscribe(
          results => {
            const contactsListResponse = <ContactsListResponse>results[0];
            const contactGroupsListResponse = <ContactGroupsListResponse>results[1];

            this.loadContacts(contactsListResponse.contacts);
            this.loadGroups(contactGroupsListResponse.contactgroups);
          },
          error => console.error(error));
    }

    loadContacts(contactIdList: string[]): void {
      Observable
        .forkJoin(
          contactIdList.map(
            contactId => this.http.get(`Contacts/${contactId}`).map(response => <Contact>response.json())
          )
        ).subscribe(
          data => this.contacts = data,
          error => console.error(error)
        );
    }

    loadGroups(groupIdList: string[]): void {
      Observable
        .forkJoin(
          groupIdList.map(
            groupId => this.http.get(`ContactGroups/${groupId}`).map(response => <ContactGroup>response.json())
          )
        ).subscribe(
          data => this.contactGroups = data,
          error => console.error(error)
        );
    }

  }

Here's the assumed structure of your models

class ContactsListResponse {
  contacts: string[];
}
class ContactGroupsListResponse {
  contactgroups: string[];
}
class Contact { }
class ContactGroup { }

C. Strategic Improvements

One question you may want to ask yourself whenever assessing a system or its part is: Does it scale [well]?

There's an issue with the way we load data. If Contacts/, ContactGroups/, Contacts/{id}, or ContactGroups/{id} API or their combination return(s) enormous amount of data, our single page application (SPA) is very likely going to screw up:

1. The application user experience cay suffer if we load too many records (scrolling through more than a few dozens of records is rarely enjoyable).

2. The application responsiveness cay be at risk if we load even more data. The numbers vary a lot, but I've seen SPAs that show noticeable lags while displaying "just 300 rows of data". That was the result of many things including heavy client-side calculation, sub-optimal code, and other...

3. The application can become unstable (may freeze or even crash the browser). If the contact/contactGroups API(s) does not limit the response at all, and returns many megabytes of data, SPA will attempt to consume whatever memory is needed to accommodate all the records.

All of these scenarios are failures from the business perspective which only vary in scale. Here are a few techniques you may want to consider as countermeasures (you can search for more).

Client-side filtering or Infinite scrolling be helpful for keeping the UX enjoyable. The infinite scrolling may be not very easy to implement very efficiently. Also, both techniques are unlikely to help with the responsiveness and stability issues in worst cases.

Server-side filtering ("search") and Pagination are a couple of effective ways to guarantee that the amount of data given to the client has a controllable limit. These things require either the API changes or client-side changes or both.

Disclaimer

I apologize in advance if my answer is too broad -- your question does not include a concrete set of aspects you'd like to get covered in review and I tried to point out some small and some bigger things.