RxJS Refactor nested map statement
Asked Answered
P

3

7

I have a service which uses @angular/http to load data from an API. I want to create a projection of the retrieved data for my Components using this data.

Therefore I wrote the following code:

getById(id: string) {
  return this.http
    .get(`https://my-api.io/${id}`)
    .map(response => response.json())
    .map(contracts =>
      contracts.map(contract =>        # <- Nested map
        new Contract(
          contract['id'],
          contract['description']
        )
      )
    );
}

In the 6th line I have a nested map-Statement reducing the readability of my code.

Question

Can I do better? Is there an operator in RxJS which I can use instead of creating this kind of nesting?

Thanks in advance!

Petrillo answered 21/10, 2016 at 6:53 Comment(4)
If I understand it correctly, there's no way to reduce the complexity here, since the outside map is of Observable and the inside is of Array type. You can consider separate it out to a function for readability though.Roan
Thanks for the quick answer. I think you are right. What I try is to transform a list into a single element that should return a list again. Extracting a method is definetly a good idea. Maybe somebody knows a more elegant way.Petrillo
@Gregor Woiwode In this case I think you need convert contracts with concatAll then you can apply .map for each contract, and combineAll Observable sequence. Note: you need return Observable, from .map, in order to use combineAll - jsbin.com/hududiq/edit?js,consoleMotorcycle
Hey I really like your approach. - It removes nesting. - There is no need to extract logic into another method.Petrillo
M
3

I propose to use flatMap/selectMany to flatten your nested array into a new stream of each single array element. In a next step you can then use RxJS.map() to do the actual mapping. Finally collect all mapped array elements with RxJS.toArray()into a new observable that provides a single array event:

const stream = $http('http://jsonplaceholder.typicode.com/posts')
 .map(res => res.data)
 .flatMap(posts => posts)
 .map(post => Object.assign({}, post, { status: false }))
 .toArray();

See sample here: http://jsbin.com/vigizihiwu/1/edit?js,console

But I would also consider: is doing such limbo really necessary? If you only have one subscriber at all, map the received array there:

const stream = $http('http://jsonplaceholder.typicode.com/posts')
 .map(res => res.data);

stream.subscribe(res => {
  const posts = res.map(post => Object.assign({}, post, { status: false }));
  console.log(posts);
});
Myriagram answered 21/10, 2016 at 22:9 Comment(1)
Thanks Marcel. I just tried it today using another sample. It turns out that I only missed to map the right property of my json returned by my API. So flatMap is definitly the way to go. And yes you are right I could map the posts inside the subscription. My goal was just to get rid of nested arrow-functions.Petrillo
P
2

Thanks to @Alexander T.. In my opinion he proposed a nice way to get around the nested map-Statement: http://jsbin.com/hududiq/1/edit?js,console

Now my code looks like the following:

getByAccountId(id: string) {
  return this.http
    .get(`http://my-api.io/${id}`)
    .map(contractsData => contractsData.json())
    .concatAll()
    .map(contract => Observable.of(new Contract(
                                    contract['id'],
                                    contract['description'])))
    .combineAll();
}
Petrillo answered 21/10, 2016 at 10:5 Comment(2)
Could you also use concatMap and do the concatenation and mapping in a single step?Schlenger
I did not tried concatMap yet. FlatMap seems to do the thing I need.Petrillo
A
2

This seems like tremendously complicated way to do a very simple thing.

If you want to avoid nested map() at all cost (even though these are two completely different methods) you can use just one map() and then forEach() on the array of results:

new Observable.of(testData)
  .map(contractsData => {
    var objects = [];
    JSON.parse(contractsData).forEach(o => objects.push(new Contract(o['id'], o['description'])));
    return objects;
  })
  .subscribe(val => console.log(val));

This will return the same result as your code.

See live demo: http://plnkr.co/edit/NQgRVSCQGgPvTkPPGz7O (I included also your code to prove the results are identical).

I'd personally use two map()s anyway because it's even more simple:

new Observable.of(testData)
  .map(contractsData => JSON.parse(contractsData).map(o => new Contract(o['id'], o['description'])))
  .subscribe(val => console.log(val));

Btw, all operators ending with *all work with higher order Observables (aka Observables emitting other Observables) and I was surprised your code works because you're passing an ordinary array to .concatAll() instead of an Observable. It turned out to be an undocumented feature of .concatAll() thanks to its inner subscriber. According to the documentation this shouldn't work.

Aec answered 21/10, 2016 at 15:43 Comment(1)
Thanks for your explanation. I was also wondering why concatAll was working. THUMBS_UPPetrillo

© 2022 - 2024 — McMap. All rights reserved.