Call service function inside constructor | Angular Community | Page 1

leaden sentinel Dec 21, 2022, 11:16 AM

#

I just want to call a service function to fullfill a variable but idk if this is the best way to do it.

constructor(private sessionService: SessionService,
    private api: ApiService,) {
    this.getCurrencies();
  }

private async getCurrencies(): Promise<Currencies[]> {
    try {
      const r = await this.api.get<Currencies[]>('/v1/configuration/currencies');
      if (r.success) {
        this._currencies = r.data;
        return r.data;
      }
      throw (r.message);
    } catch (err) {
      throw (err);
    }
  }

dire tide Dec 21, 2022, 11:33 AM

#

And where are u using the currencies variable?

#

Typically i would stick to observables, but that wouldnt change much about your question. Any reason you aren't ?

leaden sentinel Dec 21, 2022, 11:38 AM

#

dire tide And where are u using the currencies variable?

 get currencies(): Currencies[] {
    return this.companyConfig.currencies;
  }

to fill a select list box.

#

leaden sentinel Dec 21, 2022, 11:39 AM

#

dire tide Typically i would stick to observables, but that wouldnt change much about your ...

I didnt even think about observables in that case

dire tide Dec 21, 2022, 11:40 AM

#

But I would probably use

class MyService {
  currencies$ = defer(() => this.api.get<Currencies[]>('/v1/configuration/currencies')).pipe(mergemap(r => r.success ? of(r.data) : throwError(r.message)), shareReplay(1))
}

or with an ApiService that returns an Observable:

class MyService {
  currencies$ = this.api.get<Currencies[]>('/v1/configuration/currencies').pipe(mergemap(r => r.success ? of(r.data) : throwError(r.message)), shareReplay(1))
}

#

And use service.currencies$ together with the async pipe to use it in the HTML.

leaden sentinel Dec 21, 2022, 11:42 AM

#

I cant see the benefit. Could you explain?

dire tide Dec 21, 2022, 11:43 AM

#

You have data that needs to be loaded asynchronously.

#

When you kick of the async operation in the ctor of the service llike you are, you have no idea when it's done or not. You just use it and expect it will be there.

With observables, we have a mechanism that allows us to signal whenever future values arrive.

#

With this, we can leverage this with the async pipe, allowing the observable to notify the async pipe about the value being available, and the async pipe will then ensure change detection picks up this change.

#

Without that, you can have situations that, even tho the service has a value, the UI won't reflect that.

leaden sentinel Dec 21, 2022, 11:46 AM

#

I still can call on constructor, right?

dire tide Dec 21, 2022, 11:46 AM

#

The above snippet does make it lazy, meaning it wont do any HTTP calls until its available. You can easily switch this to run on service creation if you want.

dire tide Dec 21, 2022, 11:46 AM

#

leaden sentinel I still can call on constructor, right?

Not exactly like you are now, no. But you can make it run upon service creation, which has the same effect. But that would require a subscription in the service.

dire tide Dec 21, 2022, 11:47 AM

#

leaden sentinel I still can call on constructor, right?

Why do u want to call it in the constructor ?

leaden sentinel Dec 21, 2022, 11:48 AM

#

dire tide Why do u want to call it in the constructor ?

not a big deal, just didnt know another way

dire tide Dec 21, 2022, 11:48 AM

#

What do you try to achieve by calling it in the ctor ?

#

The snippet I shared allows you to use the currencies$ from all kind of places, and only the first time you use it, it will trigger an Http call.

#

All the rest will share the same response value.

#

So it will only start loading the data when u need it, but only once.

#

If you don't want that, there are ofcourse solutions for that. But often the above is more than enough.

leaden sentinel Dec 21, 2022, 11:50 AM

#

understood

dire tide Dec 21, 2022, 11:51 AM

#

I would advice to move the success part in the ApiService.

#

This would make the code more readable:

#

class MyService {
  currencies$ = this.api.get<Currencies[]>('/v1/configuration/currencies').pipe(shareReplay(1))
}

#

Now it's also clear what this does. It gets the currencies, and shares the response with all future subscribers. And keep the irrelevant succes/error mapping out of this.

dire tide Dec 21, 2022, 11:55 AM

#

leaden sentinel I cant see the benefit. Could you explain?

I think the main benefit is that this does not involve calling an async function without awaiting it.

#

But also, that it uses observables (which work well with Angular) and will prevent certain issues with CD, mostly when using OnPush on components that use the asynchronously retrieved data and are rendered before the data is available.

leaden sentinel Dec 21, 2022, 12:04 PM

#

really helpful

leaden sentinel Dec 23, 2022, 2:38 PM

#

dire tide But I would probably use ```ts class MyService { currencies$ = defer(() => th...

I did the first one

#

constructor(private wizardProductService: WizardProductService) {
    this.getConfigurationInventory();
  }

  private getConfigurationInventory() {
    this.wizardProductService.getConfigurations().pipe(map((r) => {
      r.find(config => {
        if (config.id == 1)
          this.configurationInventory = config;
      })
    })).pipe(first()).subscribe()
  }

#

Just need first value and nothing more

dire tide Dec 23, 2022, 2:45 PM

#

leaden sentinel ```ts constructor(private wizardProductService: WizardProductService) { this...

This is not what you want to do .

#

You asked for "the best way to do it", but that is definetly not one of those.

#

But feels like we are back at the start, back to calling a method in the constructor. But now with Observables.

#

I can tell you again what would be a good way to do it ... but it would be identical.

#

class YourService {
  configurationInventory$ = this.wizardProductService.getConfigurations().pipe(
    map(results => results.find(config => config.id === 1),
    shareReplay(1)
  )
}

#

I did the first one
Just to be clear, "the first one" is not at all the same as what you have.

#

Couple of things that I dont have that you do, which is not a good idea:

do not subscribe in a service
Do not set properties on the service inside map(), and definetly not inside find().

#

Your code has the exact same issue as I explained previously:

dire tide Dec 23, 2022, 2:50 PM

#

dire tide But also, that it uses observables (which work well with Angular) and will preve...

☝️

#

You are now using observables, but still Change Detection might not like that.

leaden sentinel Dec 23, 2022, 3:00 PM

#

when I use this.configurationInventory$.subscribe(x => {variable = x})
I need to unsubscribe ?

dire tide Dec 23, 2022, 3:00 PM

#

You want to try and use the async pipe.

leaden sentinel Dec 23, 2022, 3:01 PM

#

How

#

rxjs is so new for me : |

#

I can catch value using value and whatelse ?

#

getConfigurationInventory(configurationId: number): Observable<ConfigurationProductModel> {
    return this.getConfigurations().pipe(map(results => results.find(config => config.id === configurationId) as ConfigurationProductModel));
  }

class AnotherClass{
private getConfigurationInventory() {
   this.wizardProductService.getConfigurationInventory(ConfigurationsEnum.ControlProductStock).pipe(map(x =>{
    console.log(x);
    
   })).subscribe()
}

I just dont have a clue how to catch the value

#

right way

dire tide Dec 23, 2022, 3:13 PM

#

https://angular.io/guide/observables

dire tide Dec 23, 2022, 3:14 PM

#

dire tide And where are u using the currencies variable?

☝️

#

My very first question.

#

We cant be staring at code, not knowing how / where it's used.

#

But I see no reason to subscribe where u are subscribing.

#

So I would avoid that and try and use the async pipe.

#

I would advice showing what you want to achieve, and not some snippet that's pulled out of its context.

leaden sentinel Dec 23, 2022, 3:21 PM

#

dire tide I would advice showing what you want to achieve, and not some snippet that's pul...

yeah I think I took your advice for out of context and applied where it inst needed

dire tide Dec 23, 2022, 3:24 PM

#

leaden sentinel yeah I think I took your advice for out of context and applied where it inst nee...

I have no idea what you mean 😬

leaden sentinel Dec 23, 2022, 3:25 PM

#

dire tide But I see no reason to subscribe where u are subscribing.

subscribing what comes kkk

dire tide Dec 23, 2022, 3:27 PM

#

leaden sentinel subscribing what comes kkk

I am sorry, not sure what this means.

#

Happy to look into it more if you can provide more context. But based on what you shared and asked, all I can say is you dont want to do what you are doing.

leaden sentinel Dec 23, 2022, 3:34 PM

#

first of all. All my requests are promises

 get<T>(url: string, queryParams?: any): Promise<ApiResponse<T>> {
        return this.request('GET', url, null, queryParams);
    }

    post<T>(url: string, data: any): Promise<ApiResponse<T>> {
        return this.request('POST', url, data);
    }

    put<T>(url: string, data: any): Promise<ApiResponse<T>> {
        return this.request('PUT', url, data);
    }

    patch<T>(url: string, data: any): Promise<ApiResponse<T>> {
        return this.request('PATCH', url, data);
    }

    delete<T>(url: string): Promise<ApiResponse<T>> {
        return this.request('DELETE', url, null);
    }

dire tide Dec 23, 2022, 3:35 PM

#

leaden sentinel first of all. All my requests are promises ```ts get<T>(url: string, queryParam...

Any reason ?

#

Angular uses Observables everywhere, so it simplies things if u stick to Observables.

leaden sentinel Dec 23, 2022, 3:35 PM

#

not so sure. The project started as it is

dire tide Dec 23, 2022, 3:36 PM

#

I get that, but we can work around that.

#

As I showed earlier, using defer()

leaden sentinel Dec 23, 2022, 3:38 PM

#

I will start from this request

configurationSet(): Observable<ConfigurationCompanyProductModel[]> {
    try {
      return defer(() => this.api.get<ConfigurationCompanyProductModel[]>('/v1/product/config-company-product'))
        .pipe(mergeMap(r => r.success ? of(r.data) : throwError(() => r.message)), shareReplay(1))
    } catch (err) {
      throw (err);
    }
  }

As you said, I could do something that I didnt want or need

#

where I used it

canActivate(
    route: ActivatedRouteSnapshot,
    state: RouterStateSnapshot): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
    return this.wizardProductService.configurationSet().pipe(map(configuration => {
      if (configuration.length > 0){
        this.router.navigate(['/product']);
        return false;
      } 
      return true;
    }))
  }

dire tide Dec 23, 2022, 3:40 PM

#

Okay if u use it like this, there is no single reason to use a subscribe.

#

Thats why I asked about the context.

#

Also u can drop the try catch

#

The good thing is, u can also return Promises from Guards.

#

So u can avoid observables here.

#

but still doesnt need u to call anything from the ctor.

#

configurationSet(): Promise<ConfigurationCompanyProductModel[]> {
    const r = await this.api.get<ConfigurationCompanyProductModel[]>('/v1/product/config-company-product')
    if (r.success) {
       return r.data;
    } else { throw new Error(r.message)}
  }

#

canActivate(
    route: ActivatedRouteSnapshot,
    state: RouterStateSnapshot): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
    return this.wizardProductService.configurationSet().then(configuration => {
      if (configuration.length > 0){
        this.router.navigate(['/product']);
        return false;
      } 
      return true;
    })
  }

#

I would stick to Observables, but sharing that as well.

#

The downside here is, it does multiple requests.

#

While An observable can be limited to do the request once, cache the value as of then

#

I mean, you can as well with promises, but then u need to mix async and sync paradigms

#

Which gets messy

dire tide Dec 23, 2022, 3:47 PM

#

leaden sentinel where I used it ```ts canActivate( route: ActivatedRouteSnapshot, state...

Using this also does multiple requests, but that's not the code i shared. My code did a single request.

#

So my advice would be... Move the code back to how i shared it, i didnt use a method to call, but used a property.

dire tide Dec 23, 2022, 3:50 PM

#

dire tide ```ts class YourService { configurationInventory$ = this.wizardProductService....

Like that

leaden sentinel Dec 23, 2022, 3:52 PM

#

got it

#

next step I will get description from Api

leaden sentinel Dec 23, 2022, 3:53 PM

#

dire tide ``` configurationSet(): Promise<ConfigurationCompanyProductModel[]> { const ...

I think I would do the same way

leaden sentinel Dec 23, 2022, 3:54 PM

#

dire tide ``` configurationSet(): Promise<ConfigurationCompanyProductModel[]> { const ...

Is there a reason to do not use try catch ?

dire tide Dec 23, 2022, 3:59 PM

#

leaden sentinel Is there a reason to do not use try catch ?

In rxjs u use catchError

#

U already have that

leaden sentinel Dec 23, 2022, 4:00 PM

#

leaden sentinel next step I will get description from Api

async getConfigurations(): Promise<ConfigurationProductModel[]> {
    const r = await this.api.get<ConfigurationProductModel[]>('/v1/product/config-product')
    if (r.success) {
      return r.data;
    } else { throw new Error(r.message) }
  }

//All configurations for product
  private _configurations: ConfigurationProductModel[] = [];
  set setConfigurations(config: ConfigurationProductModel[]) {
     this._configurations = config;
    }
   get getAllConfigurations(): ConfigurationProductModel[] {
     return this._configurations;
   }

I need to use this more than once. I would set the value in a variable and share between components?

dire tide Dec 23, 2022, 4:01 PM

#

No, that's what we just discussed.

leaden sentinel Dec 23, 2022, 4:02 PM

#

where : s

#

Im paying attention, I aint understand some things and if you could appoint me where you said

dire tide Dec 23, 2022, 4:05 PM

#

dire tide https://angular.io/guide/observables

This

leaden sentinel Dec 23, 2022, 4:05 PM

#

in that case I didnt know the best approach so

#

I didnt get it

#

the value inst mutable

#

Why I need to watch it

dire tide Dec 23, 2022, 4:18 PM

#

Because it starts as undefined

#

Then becomes defined asynchronously

#

You want to be made aware when that happens, and cache it.

leaden sentinel Dec 23, 2022, 4:22 PM

#

constructor(private wizardProductService: WizardProductService) {
    this.getConfigurationInventory();
  }

  private getConfigurationInventory() {
   this.wizardProductService.getConfigurations().then(x => {
    x.find(x => x.id == ConfigurationsEnum.ControlProductStock ? this.configurationInventory = x : this.configurationInventory = {}  as ConfigurationProductModel)
   })
  }


 async getConfigurations(): Promise<ConfigurationProductModel[]> {
    const r = await this.api.get<ConfigurationProductModel[]>('/v1/product/config-product')
    if (r.success) {
      return r.data;
    } else { throw new Error(r.message) }
  }

why is this wrong?

#

my goal is this

dire tide Dec 23, 2022, 4:29 PM

#

You asked whether it's a good idea or not...

#

Yet you keep fighting going back to promises

#

I never said what u have wont work in some cases

#

But again, don't fire and forget async code.

#

But we are going in circles, take the time to understand observables using the link i shared.

#

That or stick with what you have.

dire tide Dec 23, 2022, 4:31 PM

#

leaden sentinel ```ts constructor(private wizardProductService: WizardProductService) { this...

It's longer, to begin with.

#

It doesnt work well with change detection and on push

leaden sentinel Dec 23, 2022, 4:34 PM

#

thanksss

#Call service function inside constructor