How to make sync call in forEach loop Angular 6

Question

I am trying to check my all 4 images is uploaded to server without any error, then redirect to another page so i am trying to perform some sync checking in my code (I have total 4 images in my imgResultAfterCompress array). below is my code:

if(Boolean(this.updateImage(data.AddId))===true)
    {
     this.router.navigate(['/job-in-hotels-india-abroad']);
    }
updateImage(AddId:number):Observable<boolean> 
{
  this.cnt=0;
  this.uploadingMsg='Uploading Images...';
        this.imgResultAfterCompress.forEach( (value, key) => {

          if(value!=='')
          {

        this.itemService.updateImage(this.employer.ID,AddId,key,value).subscribe(data=>{
          if(data && data.status == 'success') {
            this.uploadingMsg=this.uploadingMsg+'<br>Image No - '+(key+1)+' Uploaded.';  
            this.cnt++;
            }
            else
              this.alertService.error(data.message);

          });
          }
          if(this.cnt==4)
          this.uploadingDone= true;
          else
          this.uploadingDone= false
        }); 
        return this.uploadingDone;
}

Every time i am getting cnt value is 0, i want its value = 4 (completely uploaded all images) then redirection will occurred.

All your service calls inside your forEach are async. You'll reach the `cnt === 4` before all images are uploaded. You should try looking into Rxjs combination operators. Consider looking this answer https://stackoverflow.com/a/56861093/9011723 — João Ghignatti, Jul 22 '19 at 09:29

Sergey Mell · Answer 1 · 2019-07-22T12:58:00.943

2

The easier way is to wrap your observables into a single one, using zip operator

https://rxjs-dev.firebaseapp.com/api/index/function/zip

Thus once every request is finished successfully your zipped Observable will be fulfilled.

UPDATE:

This is how I think it should look like. I could miss something specific, but the global idea should be clear

    redirect() {
        this.updateImages(data.AddId).subscribe(
            () => this.router.navigate(['/job-in-hotels-india-abroad']),
            error => this.alertService.error(error.message)
        )
    }


    updateImages(AddId: number): Observable<boolean[]> {
        this.uploadingMsg = 'Uploading Images...';
        const requests: Observable<boolean>[] = [];

        this.imgResultAfterCompress.forEach((value, key) => {
            if (!value) {
                return;
            }

            requests.push(
                this.itemService.updateImage(this.employer.ID, AddId, key, value)
                    .pipe(
                        tap(() => this.uploadingMsg = this.uploadingMsg + '<br>Image No - ' + (key + 1) + ' Uploaded.'),
                        switchMap((data) => {
                            if (data && data.status == 'success') {
                                return of(true)
                            } else {
                                throwError(new Error('Failed to upload image'));
                            }
                        })
                    )
            )
        });
        return zip(...requests);
    }

edited Jul 22 '19 at 12:58

answered Jul 22 '19 at 09:30

Sergey Mell

7,780
1
26
50

can you elaborate it with my example? – amit gupta Jul 22 '19 at 10:01
Yes. Give me some time – Sergey Mell Jul 22 '19 at 10:31
@amitgupta, updated my answer with an example that could help – Sergey Mell Jul 22 '19 at 11:16
My itemService is not working if i use your code ? @SergeyMell – amit gupta Jul 22 '19 at 12:32
Actually, I've noticed that some miserable changes could be required during implementation. The code gives you an idea how it should work, but I could miss something not very important. What error do you get? – Sergey Mell Jul 22 '19 at 12:36
i am giving API call using this.itemService.updateImage() service, but its not hitting after using your code. I think .subscribe is required for this but i am not understanding where to put .subscribe in your code ? – amit gupta Jul 22 '19 at 12:43
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/196810/discussion-between-sergey-mell-and-amit-gupta). – Sergey Mell Jul 22 '19 at 12:58

score 1 · Answer 2 · answered Jul 23 '19 at 12:50

Finally got the desire result by using forkJoin

Service.ts:

public requestDataFromMultipleSources(EmpId: number,AddId:number,myFiles:any): Observable<any[]> {
    let response: any[] = [];
    myFile.forEach(( value, key ) => {
    response.push(this.http.post<any>(this.baseUrl + 'furniture.php', {EmpId: EmpId, AddId:AddId,ImgIndex:key,option: 'updateAdImg', myFile:value}));
    });
    // Observable.forkJoin (RxJS 5) changes to just forkJoin() in RxJS 6
    return forkJoin(response);
  }

my.component.ts

let resCnt=0;
this.itemService.requestDataFromMultipleSources(this.employer.ID,AddId,this.imgResultAfterCompress).subscribe(responseList => {
responseList.forEach( value => {
  if(value.status=='success')
   {
   resCnt++;
   this.uploadingMsg=this.uploadingMsg+'<br>Image No - '+(value.ImgIndex+1)+' Uploaded.';  
   }
   else
   this.uploadingMsg=this.uploadingMsg+'<br>Problem In Uploading Image No - '+(value.ImgIndex+1)+', Please choose another one.';  
 });

if(resCnt === this.imgResultAfterCompress.length)
{
  this.alertService.success('Add Posted Successfully');
  this.router.navigate(['/job-in-hotels-india-abroad']);
}
else
this.alertService.error('Problem In Uploading Your Images'); 
});

Evgenii Malikov · Answer 3 · 2019-07-22T15:51:20.037

0

You shouldn't try to make sync call within a loop. It is possible using async/await, but it's bad for app performance, and it is a common anti-pattern.

Look into Promise.all(). You could wrap each call into promise and redirect when all promises are resolved. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all

edited Jul 22 '19 at 15:51

answered Jul 22 '19 at 09:27

Evgenii Malikov

427
4
12

1

You don't need to wrap your calls into Promises, RxJs has a operator just like `Promise.all()`, take a look into `forkJoin` https://www.learnrxjs.io/operators/combination/forkjoin.html and this may help https://stackoverflow.com/a/56861093/9011723 – João Ghignatti Jul 22 '19 at 09:31
1

Observables might be too complicated for a Junior. Promise.all is easier to understand, that's why I suggested promises. – Evgenii Malikov Jul 22 '19 at 09:34
1

wait. you can make a `sync` call using `async/await`? but `async/await` freezes the app? That sounds incorrect – Jaromanda X Jul 22 '19 at 10:31
@JaromandaX Mind context. He is going to use async/await in a loop. It won't freeze the ui utself, but won't start next check execution until current one is finished. Thanks for downvoting btw. – Evgenii Malikov Jul 22 '19 at 10:41
@EvgeniiMalikov - you know what an assumption is? – Jaromanda X Jul 22 '19 at 10:42
@EvgeniiMalikov it doesn't freeze, it suspends. It certainly doesn't freeze the app. – Aluan Haddad Jul 22 '19 at 11:18
@EvgeniiMalikov it's not about wording. You stated that `async`/`await` will freeze the app. That is completely false. – Aluan Haddad Jul 22 '19 at 14:48
@AluanHaddad In my understanding it's still wording - as it blocks next code execution until previous loop cycle completed. But it doesn't freeze whole app - if you have other async code it would still be executed. I'm not a native English speaker and I don't want my answer to be misleading so I've edited my answer. – Evgenii Malikov Jul 22 '19 at 15:57
@EvgeniiMalikov I appreciate that, but the answer's still not quite right because you're saying it's bad for performance and it's an anti pattern but that neglects it's obvious use case. What it does is _serialize_ asynchronous operations on successive loop elements. Serializating does not mean blocking. You're right that it's preferable to use `Promise.all` in many circumstances. – Aluan Haddad Jul 22 '19 at 18:14
@AluanHaddad yes, sometimes there are use cases, but in general it's not a good idea to queue async calls in a loop. I've seen many bad examples of it, usually with async ajax calls where one lagged request prevents other requests from executing until it completed or failed. – Evgenii Malikov Jul 22 '19 at 18:57

How to make sync call in forEach loop Angular 6

3 Answers3