3
\$\begingroup\$

I have an Angular service consisting of three methods:

  1. This method makes a call to my server to get a URL

    private itemUrl = "http://fooserver/api/get/item/details";
    getItemDetailsUrl(productId): Observable<any> {
        return this.http
            .get(this.itemUrl, {
                params: { productId: productId }
             })
            .flatMap(res => this.getItemDetails(res.json().itemUrl))
            .map(item => item);
    }
    
  2. This takes that URL from #1 and grabs the XML data returned from the request and passes it to #3

    getItemDetails(url): Observable<any> {
        return this.http
            .get(url)
            .flatMap(item => this.parseItemFromUrl(item));
    }
    
  3. The XML passed is then converted to a JSON object using a third party library and then returned:

    parseItemFromUrl(xml): any {
        let item;
        xml2js.parseString(xml.text(), function (err, result) {
            item = result.ItemLookupResponse.Items[0].Item[0];
        });
        return Observable.of(item);
    }
    

This code does return the expected JSON object back. But since this is my first time really using Observables, I'd like to know of any improvements or changes I could make and if I used the appropriate array methods in the appropriate places.

\$\endgroup\$
  • \$\begingroup\$ Just curious, can't the api you're calling just return json instead of xml? \$\endgroup\$ – C.M. Jul 10 '18 at 6:29
  • \$\begingroup\$ I wish. The API is actually an Amazon API and unfortunately, they only return XML @C.M. \$\endgroup\$ – Noah Hornak Jul 18 '18 at 1:00
5
\$\begingroup\$

One of the improvements would be to use an HttpClient instead of Http for making the API calls.

If you use HttpClient, you won't have to call the res.json() everytime you subscribe to the response.

To use HttpClient, you'll first have to import HttpClientModule from @angular/common/http. Then you'll have to add it to the imports array of your @NgModule decorator.

Then you can inject HttpClient as a dependency and you will have access to the usual Http methods on the instance of HttpClient as well.

With that, you first part of the code will be reduced to:

private itemUrl = "http://fooserver/api/get/item/details";
getItemDetailsUrl(productId): Observable < any > {
  return this.http
    .get(this.itemUrl, {
      params: {
        productId: productId
      }
    })
    .flatMap(res => this.getItemDetails(res.itemUrl));
}

Also, I figured, there was a redundant map operation after the flatMap which wasn't really required. Rest all, I guess is in place in your code and I'm not really if it can be improved further. But I'll be looking out for answers to this question just in case.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.