0

I have a bit of code to share with you. It's from an Ionic 2 project. I think it's bad practice what I did, so I want to refactor it right.

I'm sending an image in base64 to an API, the API saves the image as .png on the server, then sends me back a response with this image's URL on the API server. The app shows a loading screen while waiting for the API response, so you can't make multiple calls in the same time. Also the API replies with 1 response for each call, and that's it.

Here's the simple call:

api.service.ts

private _imgUrl = new Subject<string>();

public getImgUrl(imgURI: any): Observable<string> {
  this.apiImgUrl(imgURI);
  return this._imgUrl.asObservable();
}

private setImgUrl(url: any) {
  this._imgUrl.next(url);
}

private apiImgUrl(imgURI: string) {
  var data  = {
    src: imgURI
  };
  var postData = JSON.stringify(data);

  var headers = new Headers({ 'Content-Type': 'application/json' });
  var options = new RequestOptions({ headers: headers });

  this.http.post(
      'my-api-url.com', 
      postData, 
      options
    )
    .retry(3)
    .toPromise()
      .then((res) => {
        var body = res.json();
        this.setImgUrl(body);
      })
      .catch(this.handleErrorPromise);
}  

my-module.ts

import { ApiService }   from '../shared';

...

private socialImageSubscription: Subscription;
private i = 0;

constructor(private _api: ApiService) {}

private getImageFromAPI(canvasURI) {
  this.socialImageSubscription = this._api.getImgUrl(canvasURI).subscribe((res:any) => {
     this.i++;
     console.log(this.i, 'socialImageSubscription', res.data);
    // doing something with the response
  });
}

Now, getImageFromAPI() is triggered by (click). So if I click once, I get this console log:

1,"socialImageSubscription", www.apidomain.com/url1

On second click:

2,"socialImageSubscription", www.apidomain.com/url2
3,"socialImageSubscription", www.apidomain.com/url2

On third click:

 4,"socialImageSubscription", www.apidomain.com/url3
 5,"socialImageSubscription", www.apidomain.com/url3
 6,"socialImageSubscription", www.apidomain.com/url4

So each time I access getImageFromAPI() it creates a new subscription, while the previous subscriptions remains active.

How can I write this code properly?

Vali Munteanu
  • 469
  • 3
  • 15
  • What do you want to achieve? Do you want to save subscriptions for multiple calls? And then kill all at once? – Amit Chigadani Nov 08 '17 at 13:37
  • @AmitChigadani Forgot to clear this out. The app shows a loading screen while waiting for the API response, so you can't make multiple calls in the same time. I don't need the subscription after I get the response. – Vali Munteanu Nov 08 '17 at 14:00
  • You don't need to kill subscriptions for http requests, angular will do it for you. https://stackoverflow.com/questions/35042929/do-you-need-to-unsubscribe-from-angular-2-http-calls-to-prevent-memory-leak – Amit Chigadani Nov 08 '17 at 14:06
  • @AmitChigadani I rewrote the question so it's clear what my problem is. Can you take another look please. – Vali Munteanu Nov 08 '17 at 14:48

2 Answers2

1

Try this : It should resolve your issue.

private getImageFromAPI(canvasURI) {
this._api.getImgUrl(canvasURI).subscribe((res:any) => {
         this.i++;
         console.log(this.i, 'socialImageSubscription', res.data);
        // doing something with the response
      }).unsubscribe();           // unsubscribe here
}

Unsubscribe it immediately, so that you don't have the subscriptions active. And also you don't need the subscription variable here.

Amit Chigadani
  • 28,482
  • 13
  • 80
  • 98
  • Nice, thanks. But is there a way to make the code even simpler, by getting rid of this part: private _imgUrl = new Subject(); public getImgUrl(imgURI: any): Observable { .... } private setImgUrl(url: any) {..... } – Vali Munteanu Nov 08 '17 at 15:11
  • Found the answer, see bellow. Thank you for help! – Vali Munteanu Nov 08 '17 at 15:26
0

Played around a bit, and found what I was looking for:

api.service.ts

private apiImgUrl(imgURI: string) {
  var data  = {
    src: imgURI
  };
  var postData = JSON.stringify(data);

  var headers = new Headers({ 'Content-Type': 'application/json' });
  var options = new RequestOptions({ headers: headers });

  return this.http.post(
      'my-api-url.com', 
      postData, 
      options
    )
    .retry(3)
    .toPromise()
      .then((res) => {
        var body = res.json();
        return body;
      })
      .catch(this.handleErrorPromise);
}  

my-module.ts

import { ApiService }   from '../shared';

...

private i = 0;

constructor(private _api: ApiService) {}

private getImageFromAPI(canvasURI) {
  this._api.apiImgUrl(canvasURI).then((res:any) => {
     this.i++;
     console.log(this.i, 'socialImageSubscription', res.data);
    // doing something with the response
  });
}

The code is much easier to read, and removes the useless subscription. The console logs are what you'd expect. First click:

1,"socialImageSubscription", www.apidomain.com/url1

On second click:

2,"socialImageSubscription", www.apidomain.com/url2

On third click:

 3,"socialImageSubscription", www.apidomain.com/url3
Vali Munteanu
  • 469
  • 3
  • 15