1

I made a service that gets some userdata from a remote source. The service a method for getting multiple users, and one for getting a specific user. The observables returned from the two methonds get .pipe(ed) thru a map() to be able to mutate the user-objects before they get consumed.

What I want is to only define the mutators once, for both the multiple users stream and the single users stream, but I run into scope problems with my current approach.

Be aware that I call users "heroes". This is a design aspect. Following are the corresponding methods from my HeroService class:

export class HeroService {
  // [...]

  getHeroes(): Observable<Hero[]> {
    return this.http.get<Hero[]>(this.heroesUrl).pipe(
      map((heroes: Hero[]) => this.mutateHeroes(heroes, this.addSkillsToHero)),
      map((heroes: Hero[]) => this.mutateHeroes(heroes, this.splitName))
    );
  }

  getHero(id): Observable<Hero> {
    return this.http.get<Hero>(this.heroesUrl + "/" + id).pipe(
      map((hero: Hero) => this.addSkillsToHero(hero)),
      map((hero: Hero) => this.splitName(hero))
    );
  }

  private mutateHeroes(heroes: Hero[], mutator) {
    heroes.forEach((hero: Hero) => {
      hero = mutator(hero);
    });
    return heroes;
  }

  private splitName(hero: Hero) {
    let heroNames: string[] = hero.name.split(" ");

    hero.firstname = heroNames.splice(0, 1).join(" ");
    hero.lastname = heroNames.splice(heroNames.length - 1, 1).join(" ");
    hero.middlename = heroNames.join(" ");

    return hero;
  }

  private addSkillsToHero(hero: Hero) {
    hero.skills = this.getSkills(Math.ceil(Math.random() * 10));
    return hero;
  }

  private getSkills(count: number): string[] {
    let skills: string[] = [];
    let i;

    for (i = 0; i < count; i++) {
      skills.push(this.getRandomSkill());
    }

    return skills;
  }

  private getRandomSkill(): string {
    return SKILL_TAGS[Math.floor(Math.random() * SKILL_TAGS.length)];
  }
  // [...]
}

The catchError() of the Observable(s) return: Cannot read property 'getSkills' of undefined I am suspecting that the mutator does not get called inside the class scope, and cant be found therefore.

How would I do such a thing in JS?

The whole project can be inspected at:

DumperJumper
  • 75
  • 11

2 Answers2

0

The problem is here:

 getHeroes(): Observable<Hero[]> {
    return this.http.get<Hero[]>(this.heroesUrl).pipe(
      map((heroes: Hero[]) => this.mutateHeroes(heroes, this.addSkillsToHero)),
      map((heroes: Hero[]) => this.mutateHeroes(heroes, this.splitName))
    );
  }

Passing this.addSkillsToHero as this, doesn't keep the this reference. You are passing a reference to a function, this is determined at call time. You either need to bind that function to the this you need, or use an arrow function which does the capturing of this for you. You can find more details here: JavaScript function binding (this keyword) is lost after assignment or https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this

You can try it like:

 getHeroes(): Observable<Hero[]> {
    return this.http.get<Hero[]>(this.heroesUrl).pipe(
      map((heroes: Hero[]) => this.mutateHeroes(heroes, this.addSkillsToHero.bind(this))),
      map((heroes: Hero[]) => this.mutateHeroes(heroes, this.splitName.bind(this)))
    );
  }

or using arrow functions:

 getHeroes(): Observable<Hero[]> {
    return this.http.get<Hero[]>(this.heroesUrl).pipe(
      map((heroes: Hero[]) => this.mutateHeroes(heroes, h => this.addSkillsToHero(h))),
      map((heroes: Hero[]) => this.mutateHeroes(heroes, h => this.splitName(h)))
    );
  }
Andrei Tătar
  • 7,872
  • 19
  • 37
0

Arrow Functions (Lamdas) to the Rescue

As already mentioned, your issue comes down to how a function's context (this) is decided. This is something that is done at run-time.

To expand on another answer given here, you can define a function with its context (this) already bound. The trick is to invoke the function early and work with a partially applied version of that function. One way to do that is with bind.

const functionName = (function (args){
  /* Some code */
  return /*something*/
}).bind(this);

That's kinda ugly, so JavaScript has arrow functions that do much the same thing.

const functionName = (args) => {
  /* Some code */
  return /*something*/
}

That's much prettier. Even better is that one-line single-expression Lamdas have an implicit return:

const functionName = (args) => {
  return /*something*/;
}
// Is the same as
const functionName = (args) => /*something*/;

Normal functions are given a context when invoked. Arrow functions always bind the scope they're defined in as their context. If you're not using Javascript's prototypal inheritance, there's generally not much reason that a function's context would need to change from the scope it is defined in.

I find you can really neaten up your code if you're willing to lean into this knowledge a bit.

Here's how I would write your HeroService class entirely with arrow functions. If this is the best way to do this is up for debate. Certainly, this approach allows for more of a declarative/functional style. In practice, a mix of the two gets you the best of both worlds.

export class HeroService {
  // [...]

  public getHeroes = (): Observable<Hero[]> =>
    this.http.get<Hero[]>(this.heroesUrl).pipe(
      map(this.mutateHeroes(this.addSkillsToHero)),
      map(this.mutateHeroes(this.splitName))
    );

  public getHero = (id): Observable<Hero> => 
    this.http.get<Hero>(this.heroesUrl + "/" + id).pipe(
      map(this.addSkillsToHero),
      map(this.splitName)
    );

  private mutateHeroes = transformer => 
    (heroes: Hero[]) => heroes.map(transformer);

  private splitName = (hero: Hero) => {
    let heroNames: string[] = hero.name.split(" "); 
    return ({
      ...hero,
      firstName: heroNames.splice(0, 1).join(" "),
      lastname: heroNames.splice(heroNames.length - 1, 1).join(" "),
      middlename: heroNames.join(" ")
    });
  }

  private addSkillsToHero = (hero: Hero) => ({
    ...hero, skills: this.getSkills(Math.ceil(Math.random() * 10))
  });

  private getSkills = (count: number): string[] => 
    Array.from({length: count}).map(this.getRandomSkill);

  private getRandomSkill = (): string => 
    SKILL_TAGS[Math.floor(Math.random() * SKILL_TAGS.length)];
  
  // [...]
}

Extra: Composing Functions

A cool property of a properly implemented map function (does not matter if you're mapping over arrays or streams) is that:

map(x => f(g(x))) 
  === 
map(g).map(f)

This tells us that if we compose addSkillsToHero, and splitName then you can do this with a single map instead of two. RxJS streams are actually transducers, so they do this for you (in a way), but array map doesn't so you can gain some marginal performance through compositions as well (You iterate through your array only once!).

Normally you'd you a library like Ramda/Redux/Lodash/etc and you could write something like

private addSkillsAndSplitName = 
  compose(this.addSkillsToHero, this.splitName);

But composing two functions is pretty easy with vanilla JavaScript too.

private addSkillsAndSplitName = 
  hero => this.addSkillsToHero(this.splitName(hero))

Then you can use this new function like this:

export class HeroService {
  // [...]

  public getHeroes = (): Observable<Hero[]> =>
    this.http.get<Hero[]>(this.heroesUrl).pipe(
      map(this.mutateHeroes(this.addSkillsAndSplitName))
    );

  public getHero = (id): Observable<Hero> => 
    this.http.get<Hero>(this.heroesUrl + "/" + id).pipe(
      map(this.addSkillsAndSplitName)
    );

And you haven't taken a hit on testability, flexibility, or separation of concerns as the functionality still exists in separate functions.

Mrk Sef
  • 7,557
  • 1
  • 9
  • 21