Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is it meant to be used in dependency injection or not? #26

Open
tasutafat opened this issue Feb 24, 2023 · 6 comments
Open

Is it meant to be used in dependency injection or not? #26

tasutafat opened this issue Feb 24, 2023 · 6 comments
Assignees

Comments

@tasutafat
Copy link

Hello there, I was just wondering if it makes sense to create one Translator and use dependency injection to get it to the classes it's needed in or if there should be a new Translator each time it is needed?

Best regards,
tasutafat

@tasutafat tasutafat changed the title Is it meant to use in dependency injection or not? Is it meant to be used in dependency injection or not? Feb 24, 2023
@JanEbbing
Copy link
Member

Hello,
on a technical level: The overhead to create multiple Translators itself is pretty low, the only thing is that multiple Translator objects would create multiple HTTP sessions (which is a bit of overhead).
I think the main question here is just an architectural decision. By default I would lean towards creating a single Translator which you can expose to your Application as a singleton or something similar.

@tasutafat
Copy link
Author

Thank you very much for your response! I will add it to my Service collection!

@antoniovalentini
Copy link

antoniovalentini commented Mar 5, 2023

@daniel-jones-deepl, @JanEbbing, would you evaluate the possibility to add some service collection extensions? I wrote a small POC in this PR #28.

@DeeJayTC DeeJayTC self-assigned this Apr 17, 2023
@DeeJayTC
Copy link
Member

DeeJayTC commented Apr 18, 2023

Hey @antoniovalentini

Is this what you had in mind?

https://github.com/DeepLcom/deepl-dotnet/blob/issue-26%2B29/src/DeepL.Service/DeepLService.cs

How to use it:

builder.Services.AddDeepL(options => {

@antoniovalentini
Copy link

antoniovalentini commented Apr 21, 2023

Hey @DeeJayTC, yes that's what I had in mind. Since you're integrating this into the development branch, I think my PR is not useful anymore. Should I close it?

@DeeJayTC
Copy link
Member

Yea, i alread had it sitting here unfinished. Your PR reminded me to get it done! 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants