HiveBrain v1.2.0
Get Started
← Back to all entries
patterntypescriptMinor

Making multiple http request Angular2

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
angular2requesthttpmultiplemaking

Problem

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);
  }
});
}

Solution

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: 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.

  1. 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 = results[0];
            const 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 => 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 => 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:

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

-
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 dis

Code Snippets

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)
        );
    }

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

Context

StackExchange Code Review Q#160077, answer score: 2

Revisions (0)

No revisions yet.