0

I am trying to check the validity of a function I have written in Typescript, in congruence with RxJS observables, that fetches some bookings from one service and then for each booking fetches its corresponding location and activity from another service.

I am simply writing this post to verify the validity of what I have written and to ask if there is anything I could have done more efficiently.

let params = new HttpParams();
params = params.append('status', 'C');
params = params.append('offset', offset.toString());
params = params.append('limit', limit.toString());
return this.http.get(`${this.environment.booking.url}/my/bookings`, { params }).pipe(
    mergeMap((bookings: Booking[]) => {
        if(bookings.length > 0) {
            return forkJoin(
                bookings.map((booking: Booking) =>
                    forkJoin(
                        of(booking),
                        this.activityService.getActivity(booking.activity),
                  this.locationService.getLocation(booking.finalLocation),
                    ).pipe(
                        map((data: [ Booking, Activity, Location ]) => {
                            let booking = data[0];
                            booking.activityData = data[1];
                            booking.finalLocationData = data[2];
                            return booking;
                        })
                    )
                )
            )
        }

        return of([]);
    }),
    catchError((err: HttpErrorResponse) => throwError(err))
);

I am expecting for this function to return a list of bookings alongside their corresponding location and activity. However more importantly I want to verify that what I am doing is correct and sensible. Is there anything I could have done differently to make it cleaner/ more human-readable (not nit-picking, please )?

On a different note, that of performance, I also have a follow-up question with regards to performance. Given that a list of bookings has common activities and locations. Is there a way to only fetch activities and locations without any duplicate HTTP requests? Is this already handled under the hood by RxJS? Is there anything I could have done to make this function more efficient?

VKen
  • 4,964
  • 4
  • 31
  • 43

2 Answers2

0

This is how I would tackle this using RxJS:

  1. Fetch all the Bookings
  2. For Each Booking fetch Location and Activities cuncurrently

const { from, of, forkJoin, identity } = rxjs;
const { mergeMap, tap, catchError } = rxjs.operators;

const api = 'https://jsonplaceholder.typicode.com';

const endpoints = {
  bookings: () => `${api}/posts`,
  locations: (id) => `${api}/posts/${id}/comments`,
  activities: (id) => `${api}/users/${id}`
};

const fetch$ = link => from(fetch(link)).pipe(
  mergeMap(res => res.json()),
  catchError(() => from([])),
);

fetch$(endpoints.bookings()).pipe(
  mergeMap(identity),
  mergeMap(booking => forkJoin({
    booking: of(booking),
    locations: fetch$(endpoints.locations(booking.id)),
    activities: fetch$(endpoints.activities(booking.userId)),
  })),
).subscribe(console.log);
<script src="https://cdnjs.cloudflare.com/ajax/libs/rxjs/6.5.3/rxjs.umd.js" integrity="sha256-Nihli32xEO2dsnrW29M+krVxoeDblkRBTkk5ZLQJ6O8=" crossorigin="anonymous"></script>

note:

  1. Reactive Programming, and more generically declarative approaches, focus on avoiding imperative control flows... You should try to write your pipes without conditions (or any other control flow). To discard empty bookings you can use the filter operator.
  2. Avoid nesting streams because this comes at the cost of readability.
  3. forkJoin also takes a spec object which is very useful (part of the overloads)
Hitmands
  • 13,491
  • 4
  • 34
  • 69
0

I'm not sure about the efficiency, but, at least for me, it was a little hard to read

Here's how I'd do it:

I used a dummy API, but I think it correlates with your situation

const usersUrl = 'https://jsonplaceholder.typicode.com/users';
const todosUrl = 'https://jsonplaceholder.typicode.com/todos';
const userIds$ = of([1, 2, 3]); // Bookings' equivalent

userIds$
  .pipe(
    filter(ids => ids.length !== 0),
    // Flatten the array so we can avoid another nesting level
    mergeMap(ids => from(ids)),
    // `concatMap` - the order matters!
    concatMap(
      id => forkJoin(ajax(`${usersUrl}/${id}`), ajax(`${todosUrl}/${id}`))
        .pipe(
          map(([user, todo]) => ({ id, user: user.response, todo: todo.response }))
        )
    ),
   toArray()
  )
  .subscribe(console.log)

Here is a StackBlitz demo.

With this in mind, here is how I'd adapt it to your problem:

this.http.get(`${this.environment.booking.url}/my/bookings`, { params }).pipe(
    filter(bookings => bookings.length !== 0),
    // Get each booking individually
    mergeMap(bookings => from(bookings)),
    concatMap(
        b => forkJoin(
            this.activityService.getActivity(b.activity),
            this.locationService.getLocation(b.finalLocation),
        )
        .pipe(
            map(([activity, location]) => ({ ...b, activity, location }))
        )
    ),
    // Getting the bookings array again
    toArray()
    catchError((err: HttpErrorResponse) => throwError(err))
);
Andrei Gătej
  • 11,116
  • 1
  • 14
  • 31