0

Say I have this interface:

public interface IExample
{
    string GetSupportedExtension();

    void DoWork(Stream input);
}

a concrete implementation like:

public class PdfExample : IExample
{
    public string GetSupportedExtension() => ".pdf";

    public void DoWork(Stream input)
    {
        // Do something with the PDF
    }
}

A Factory/Resolver:

public class ExampleResolver(IEnumerable<IExample> examples) : IExampleResolver
{
    public IExample? Resolve(string extension)
    {
        return examples.FirstOrDefault(e => e.GetSupportedExtension() == extension);
    }
}

Used in a controller like this:

[ApiController]
public class ExampleController(IExampleResolver resolver) : ControllerBase
{
    [HttpPost("/")]
    public async Task<IActionResult> DoWork([FromForm] IFormFile file)
    {
        string extension = Path.GetExtension(file.FileName).ToLower();
        IExample? example = resolver.Resolve(extension);

        if (example == null)
        {
            return BadRequest();
        }

        example.DoWork(file.OpenReadStream());

        return Ok();
    }
}

Which I thought was a class/interface structure that made sense and let the DI framework construct the objects for me, however there's a problem if I have another concrete implementation:

public class ZipExample(IExampleResolver resolver) : IExample
{
    public string GetSupportedExtension() => ".zip";

    public void DoWork(Stream input)
    {
        // Load zip from stream

        foreach (var file in zip)
        {
            IExample example = resolver.Resolve(file.Extension);
            example.DoWork(file.GetInputStream());
        }
    }
}

Now there's a circular reference as the resolver needs the ZipExample to be constructed but the ZipExample also needs the resolver.

I know other questions asked similar to this would mention refactoring to remove the circular dependency but I struggle to see a good way of doing that here with the circular/recursive nature of what I need to do, any suggestions?

I also know I could pass in IServiceProvider instead but read it was bad practice as it hides a class's dependencies so I guess I'm looking to see if there's another solution or what people would recommend? Thanks

3
  • 2
    Why does the ZipExample need the resolver (in the constructor)? At least I can't see the reason from this example. Commented Sep 22 at 14:41
  • I want to extract the zip and loop through each file in the zip and get the right IExample per extracted file Commented Sep 22 at 14:45
  • 2
    A possible solution would be to add ZipExample as a special case in your ExampleResolver. And while passing a IServiceProvider should normally be avoided, it might be the better than the alternatives in some cases. While DI is useful, it is sometimes simpler to do something else, like using an explicit factory. Commented Sep 22 at 14:47

3 Answers 3

2

One way to break the cycle would be to implement resolver in following way:

public class ExampleResolver(IServiceProvider serviceProvider) : IExampleResolver
{
    public IExample? Resolve(string extension)
    {
        var examples = serviceProvider.GetRequiredService<IEnumerable<IExample>>();
        return examples.FirstOrDefault(e => e.SupportsExtension(extension));
    }
}

And I also would suggest defining supported method extension in slightly cleaner way:

public bool SupportsExtension(string extension)
{
    return extension == "zip";
}

If your implementation have state, i would register them as Transient, as otherwise if implementations are resolved in the same scope, the same instances would be reused. But it's up to you to find the best strategy.

Sign up to request clarification or add additional context in comments.

1 Comment

I would say that for this particular use case, the answer you provided is the answer. I don't see any other practical way to break the cycle.
2

I would suggest that you consider your structure here. The Pdf example 'does work' with a single file in your stream. From the rest of the example, it seems like this is the general rule for files.

The zip code extracts all the files in the archive and then 'does work' on each one. 'And' is rarely a good thing in a method name 🙂

They're doing two different things, you never actually DoWork on the zip file itself, just the files within.

This sounds like a different bit of code is needed to handle compressed files, rather than shoehorning into the structure you have.

Comments

0

By injecting IEnumerable<IExample> into ExampleResolver in any way, you are forcing DI to construct all IExample services, even when you only intend to use one. Which is a waste if the services aren't Singleton's.

Instead capture the mapping between extension and implementation type while you register each service. Use a static abstract interface for GetSupportedExtension to obtain the extension without constructing anything. And use the correct StringComparer so you don't have to convert anything to lower case.

public interface IExample
{
    void DoWork(Stream input);
    static abstract string GetSupportedExtension();
}

public class ExampleMappings : Dictionary<string, Type>
{
    private readonly IServiceCollection services;
    public ExampleMappings(IServiceCollection services) : base(StringComparer.OrdinalIgnoreCase) { 
        this.services = services;
    }

    public ExampleMappings Register<T>() where T : class, IExample
    {
        this.Add(T.GetSupportedExtension(), typeof(T));
        services.AddScoped<T>();
        return this;
    }
}

public class ExampleResolver(IServiceProvider sp, ExampleMappings mappings) : IExampleResolver
{
    public IExample? Resolve(string extension)
    {
        if (!mappings.TryGetValue(extension, out var type))
            return null;

        return (IExample)sp.GetService(type);
    }
}

public static ExampleMappings RegisterResolver(this IServiceCollection services) 
{
    services.AddScoped<ExampleResolver>();
    var mappings = new ExampleMappings();
    services.AddSingleton(mappings);
    return mappings;
}

services.RegisterResolver()
    .Register<PdfExample>()
    .Register<OtherExample>();

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.