Configure using the options pattern

[jeffdahl]

The options pattern simplifies configuration. When using a settings.json file, instead of configuring GoogleAds with the following three lines:

IConfigurationSection section = Configuration.GetSection("GoogleAdsApi");
GoogleAdsConfig config = new GoogleAdsConfig(section);
GoogleAdsClient client = new GoogleAdsClient(config);

configuration can be done in a single line:

using IHost host = Host.CreateDefaultBuilder(args)
    .ConfigureServices((hostContext, services) =>
        services.AddGoogleAdsClient(hostContext.Configuration))
    .Build();

[AnashOommen]

None of GoogleAdsConfig, GoogleAdsClient and services are designed to be singletons, because that's not how the product works in the general case.

I'll give it some thought, but perhaps it is simpler to have a SingletonGoogleAdsClientProvider helper class specifically for this use case. @Raibaz wdyt?

[jeffdahl]

Good to know. Instead of singletons, we could register GoogleAdsConfig and GoogleAdsClient as transient. Would that fit the design better?

On Tue, Jan 17, 2023 at 8:31 PM Anash P. Oommen @.***> wrote:

None of GoogleAdsConfig, GoogleAdsClient and services are designed to be singletons, because that's not how the product works in the general case.

I'll give it some thought, but perhaps it is simpler to have a SingletonGoogleAdsClientProvider helper class specifically for this use case. @Raibaz https://github.com/Raibaz wdyt?

— Reply to this email directly, view it on GitHub https://github.com/googleads/google-ads-dotnet/issues/483#issuecomment-1386472553, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVHMB5ZDFOM237YZK35UV3WS5W2HANCNFSM6AAAAAAT6C42BY . You are receiving this because you authored the thread.Message ID: @.***>

[Raibaz]

Not entirely sure if registering GoogleAdsConfig and GoogleAdsClient as transient would be feasible; I'd rather keep them as they are and instead add a Singleton to wrap them for dependency injection usage.

[AnashOommen]

@jeffdahl I think @Raibaz and I need some more time thinking through this. How about we accept this bug for ourselves, and send you a PR when we are ready? I'd be happy to credit you for the work even if we end up with a different implementation.

[jeffdahl]

Sure, sounds good. Thanks for considering my suggestion.

Jeff

On Tue, Feb 7, 2023, 8:58 AM Anash P. Oommen @.***> wrote:

@jeffdahl https://github.com/jeffdahl I think @Raibaz https://github.com/Raibaz and I need some more time thinking through this. How about we accept this bug for ourselves, and send you a PR when we are ready? I'd be happy to credit you for the work even if we end up with a different implementation.

— Reply to this email directly, view it on GitHub https://github.com/googleads/google-ads-dotnet/issues/483#issuecomment-1421105343, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVHMBYOVM4BLVEHCLDVBC3WWJ5LZANCNFSM6AAAAAAT6C42BY . You are receiving this because you were mentioned.Message ID: @.***>

[jeffdahl]

Any consideration of adding the two interfaces in PR #540? As explained in the PR, adding these two interfaces will allow a configured IGoogleAdsClient to be injected via the DI container.

Thanks for considering.