I have the following code, wherein I am pulling data from a variety of sources and setting a loaded flag once it is done. At the moment however, it is rather cumbersome to maintain. How can I go about fixing it?
constructor(
private courseContentFacade: CourseContentFacade,
private changeDetectorRef: ChangeDetectorRef
) {
this.courseContentFacade
.getUnsplashPhotos()
.pipe(takeUntil(this.ngUnsubscribe))
.subscribe((unsplashPhotos) => {
this.unsplashPhotosLoaded = false;
if (unsplashPhotos) {
this.parseUnsplashPhotos(unsplashPhotos);
}
this.courseContentFacade
.getPexelsPhotos()
.pipe(takeUntil(this.ngUnsubscribe))
.subscribe((pexelsPhotos) => {
this.unsplashPhotosLoaded = false;
if (pexelsPhotos) {
this.parsePexelsPhotos(pexelsPhotos);
}
setTimeout(
() => {
this.courseMediaItemsLoaded = true;
this.pexelsPhotosLoaded = true;
this.unsplashPhotosLoaded = true;
},
environment.production ? 2500 : 1000
);
});
});
}
Here is how I might rework this to be a bit more idiomatic RxJS. This code still leans heavily on the use of global state.
The less your application as a whole requires mutable global state, the easier it will be to maintain and extend. This is 5x - 10x as true when it comes to RxJS.
Even so, using higher-order operators (in this case mergeMap) does a lot to cut down on the nesting and complexity created here :)
constructor(
private courseContentFacade: CourseContentFacade,
private changeDetectorRef: ChangeDetectorRef
) {
this.courseContentFacade.getUnsplashPhotos().pipe(
tap(_ => this.unsplashPhotosLoaded = false),
tap(unsplashPhotos => {
if (unsplashPhotos) {
this.parseUnsplashPhotos(unsplashPhotos);
}
}),
mergeMap(_ => this.courseContentFacade.getPexelsPhotos()),
tap(_ => this.pexelsPhotosLoaded = false),
tap(pexelsPhotos => {
if(pexelsPhotos) {
this.parsePexelsPhotos(pexelsPhotos);
}
}),
takeUntil(this.ngUnsubscribe),
delay(environment.production ? 2500 : 1000)
).subscribe(_ => {
this.courseMediaItemsLoaded = true;
this.pexelsPhotosLoaded = true;
this.unsplashPhotosLoaded = true;
});
}