4

I have a class that when initialized retrieves data from a service and populates one of its properties, which is an array. This same class has a function that sorts, filters and returns this array. When I instantiate an object of this class and call this function I realized it's called before its constructor and ngOnInit() functions are done (probably because I use async content from the Observables the service returns). How can I guarantee constructor and init have completely executed before any function of my class are called externally?

    export class BaseChoice implements PickAppraiser, OnInit {
weight = 0;
options = new Array<PickQuality>();

constructor(private championService: ChampionService) {}

ngOnInit() {
    // Iterates through the list of champions adding them to the current object
    this.championService.getChampions()
        .subscribe(champions => {
            // Iterates through the list of champions adding them to the current object
            Object.keys(champions).map(key => this.options.push(new PickQuality(champions[key], 0)))
        })
}

choose(n?: number): PickQuality[] {
    var sorted = this.options.sort((a, b) => a.score - b.score);
    return sorted;
}

}

I also tried to do something like

    choose(n?: number): PickQuality[] {
    // Iterates through the list of champions adding them to the current object
    this.championService.getChampions()
        .subscribe(champions => {
            // Iterates through the list of champions adding them to the current object
            Object.keys(champions).map(key => this.options.push(new PickQuality(champions[key], 0)))
            this.reevaluate(this.options);

            var sorted = this.options.sort((a, b) => a.score - b.score);
            var chosen;
            if(n) chosen = sorted.slice(1, n);
            else chosen = sorted.slice(1, 2);
            return chosen;
        });
}

where I run the async request inside the choose() method itself, but it won't let me do so, I assume because the return variable is not guaranteed to exist.

Luis Paulo
  • 129
  • 2
  • 10
  • This would depend on how the content is being called externally. Is it being called by a parent component, a directive, a service, etc? Any reason you can't sort the list after the map function? – joh04667 Feb 06 '17 at 20:11
  • Have a look at [Is it bad practice to have a constructor function return a Promise?](http://stackoverflow.com/q/24398699/1048572). Don't do anything asynchronous in the initialisation of an instance (via the constructor directly or angular hooks), do it before creating the instance. – Bergi Feb 06 '17 at 20:26
  • How do I do anything before the instance creation in a class? Isn't the constructor the first thing that runs in a class? – Luis Paulo Feb 06 '17 at 21:00
  • The easiest (though not most elegant) thing you can do is not initialize the options when you're making ur class. Then in your template you can do `options?.doSomething()` or `*ngIf(options)` – mrBorna Feb 06 '17 at 21:29

3 Answers3

1

I think, you should look at how you're fundamentally laying out your component. You can take advantage of observables the way they were meant to be used as angular supports it in your template with the async pipe.

I'm not sure of the details of your component, but I'd do something like this:

export class BaseChoice implements PickAppraiser, OnInit {
    weight = 0;
    options$: Observable<PickQuality>;
    champions$ : Observable<Champion>;

    constructor(private championService: ChampionService) {}

    ngOnInit() {
        this.champions$ = this
            .championService.getChampions();

        this.options$ = this.champions$.map((champion, index) => {
          return new PickQuality(champion, 0)))
      })
    }
}

In your template, If you do *ngFor="let option in options$ | async) it will automatically run that stream and give you the result, then in your choose() function which I'm assuming is an action a user takes on a click, you can just directly pass the option to do something with it.

If it's more complicated than that, you can map that to a stream of clicks like championClicked$ and just map those clicks to the correct option from the options stream.

The thing to keep in mind is that you're setting up these observables with a pipeline of actions, and that that pipeline runs once per Observer (subscriber) which means every time you use the | async pipe it's subscribing and running the whole thing.

Spending some time learning about RxJS will pay off huge in your Angular 2 development.

mrBorna
  • 1,757
  • 16
  • 16
0

Since you are initializing the options property to an empty array options = new Array<PickQuality>();, why not put a check in the choose method?

choose(n?: number): PickQuality[] {
    if (this.options && this.options.length !== 0) {
       var sorted = this.options.sort((a, b) => a.score - b.score);
       return sorted;
    } else {
       /* do something else if options is empty */
    }
}

--- Edit ---

Here is a roughed out version of what I mentioned in the comments:

export class BaseChoice implements PickAppraiser, OnInit {
weight = 0;
options = new Subject<PickQuality[]>();

constructor(private championService: ChampionService) {}

ngOnInit() {
    this.championService.getChampions()
        .subscribe(champions => {
            Let arr = [];
            Object.keys(champions).map(key => arr.push(new PickQuality(champions[key], 0)));
             this.options.next(arr);
        })
}

choose(n?: number): PickQuality[] {
    return this.options.take(1).sort((a, b) => a.score - b.score);
}
}
JRulle
  • 7,448
  • 6
  • 39
  • 61
  • What would I do on else statement? I can't just dismiss the choose() function call just because the async content hasn't run yet. Nor can I stay on busy wait (like while(1)) cause then my whole app stops. – Luis Paulo Feb 06 '17 at 21:02
  • That would be dependent on what fires the choose method and what it is doing (you pass in 'n' but it is not used). In my apps I would actually create a RXJS Subject. options = new Subject (); Then in choose apply your sort and return the observable so your recipient of the choose method's response would wait (async) for the array. – JRulle Feb 06 '17 at 21:05
  • I actually use it in to split my array like options.split(0,n). I'll check out this Subject thing, thanks. – Luis Paulo Feb 06 '17 at 21:08
  • I can modify my answer above later but I'm on my mobile right now. – JRulle Feb 06 '17 at 21:08
0

If you are calling choose(n) from a template you could add a *ngIf="options" to the tag in the template. You'd have to change how you declare "options" so it is undefined until the data is loaded. Something like this:

options: PickQuality[];

If you are calling choose(n) from a service, I would change it to return an Observable, or Promise instead.

Per
  • 491
  • 6
  • 19