patterntypescriptMinor
Making multiple http request Angular2
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
Use explicit types wherever possible. This will:
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
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.
TypeScript is a superset of JavaScript, i.e. whatever is syntactically correct in JS is also correct in TS.
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.
Here's the assumed structure of your models
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
-
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
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
- 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.
- 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 withconstand switch toletonly 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 asconst. 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.