[Suggestion] Generic IServiceFactory to reduce repetitive code

[adearriba]

Hello,

This is just a proposal to have a generic IServiceFactory interface instead of individual factory interfaces.

internal interface IServiceFactory<T>
    where T: IShopifyService
{
    /// Creates a new instance of the <see cref="T" /> with the given credentials.
    /// <param name="shopDomain">The shop's *.myshopify.com URL.</param>
    /// <param name="accessToken">An API access token for the shop.</param>
    T Create(string shopDomain, string accessToken);

    /// Creates a new instance of the <see cref="T" /> with the given credentials.
    /// <param name="credentials">Credentials for authenticating with the Shopify API.</param>
    T Create(ShopifyApiCredentials credentials);
}

The template will change to:

public class @@REPLACEME@@Factory(IRequestExecutionPolicy? requestExecutionPolicy = null, IShopifyDomainUtility? shopifyDomainUtility = null) : IServiceFactory<I@@REPLACEME@@>
{
    /// <inheritDoc />
    public virtual I@@REPLACEME@@ Create(string shopDomain, string accessToken)
    {
        I@@REPLACEME@@ service = shopifyDomainUtility is null ? new @@REPLACEME@@(shopDomain, accessToken) : new @@REPLACEME@@(shopDomain, accessToken, shopifyDomainUtility);

        if (requestExecutionPolicy is not null)
        {
            service.SetExecutionPolicy(requestExecutionPolicy);
        }

        return service;
    }

    /// <inheritDoc />
    public virtual I@@REPLACEME@@ Create(ShopifyApiCredentials credentials) =>
        Create(credentials.ShopDomain, credentials.AccessToken);
}

[nozzlegear]

@adearriba Thanks for the suggestion! I like that a lot, I think I may have even had something similar in a git stash that I'd been experimenting with as well. The only thing I'm concerned about is the PartnerService, which uses the ShopifyPartnerCredentials class instead of ShopifyApiCredentials.

I'm not sure how we'd make it work so that you can't use IServiceFactory<PartnerService>, as calling IServiceFactory<PartnerService>.Create(ShopifyApiCredentials) shouldn't be possible. Any suggestions?

[adearriba]

Thanks, @nozzlegear!

The PartnerService is special as it doesn't use the same parameters. Instead of string shopDomain it uses long partnerOrganizationId. I think it's best to have it separately.

The current template doesn't work for PartnerService either because of this detail. Maybe if there are more Partner Services in the future, it makes sense to create a separate interface for them. Currently, as it's only one special service, I think it can be left as it is.

public interface IPartnerServiceFactory
{
    /// Creates a new instance of the <see cref="IPartnerService" /> with the given credentials.
    /// <param name="partnerOrganizationId">Your Shopify Partner organization ID. This can be found on your Shopify Partner account settings page.</param>
    /// <param name="accessToken">An API access token for the shop.</param>
    IPartnerService Create(long partnerOrganizationId, string accessToken);

    /// Creates a new instance of the <see cref="IPartnerService" /> with the given credentials.
    /// <param name="credentials">Credentials for authenticating with the Shopify API.</param>
    IPartnerService Create(ShopifyPartnerApiCredentials credentials);
}

The other option is to enforce the credentials constructor and remove the deconstructed values and use two generics for the interface, but I think this is going to far for just 1 outlier.

internal interface IServiceFactory<TInterface, TCredentials>
    where TInterface: IShopifyService
    where TCredentials : class
{
    /// Creates a new instance of the <see cref="TInterface" /> with the given credentials.
    /// <param name="credentials">Credentials for authenticating with the Shopify API.</param>
    TInterface Create(TCredentials credentials);
}

[nozzlegear]

Awesome, thanks for giving your thoughts! I like the first option I think, it doesn't make sense to go through all that extra work for just one outlier with the PartnerService.