IEnumerable allow circular dependencies

[Scrawach]

Let's say I want to do something wrong:

Example 1. Not working.

public class Greater
{
   private readonly DebugConsole _console;

   public Greater(DebugConsole console) => 
        _console = console;
    
   public void Hello() => 
        _console.Notify("Hello, World!");
}

public class DebugConsole
{
    private readonly Greater _greater;

    public DebugConsole(Greater greater) => 
        _greater = greater;
    
    public void Welcome() => 
        _greater.Hello();

    public void Notify(string message) => 
        Debug.Log(message);
}

var builder = new ContainerBuilder();
builder.AddSingleton(typeof(DebugConsole));
builder.AddSingleton(typeof(Greater));
var container = builder.Build();
var console = container.Resolve<DebugConsole>();
console.Welcome();

There is a circular dependency here (Greater <-> DebugConsole) and the container, expectedly, can't solve this problem. In runtime, we'll get an exception, when try resolve it, refactor the code, use other approaches to solve it, and solve this problem, making our code better. That's fine. However, I can hack it with enumerations:

Example 2. Working.

public class Greater
{
   private readonly DebugConsole _console;

   public Greater(DebugConsole console) => 
        _console = console;
    
   public void Hello() => 
        _console.Notify("Hello, World!");
}

public class DebugConsole
{
    private readonly IEnumerable<Greater> _greater;

    public DebugConsole(IEnumerable<Greater> greater) => 
        _greater = greater;
    
    public void Welcome() => 
        _greater.First().Hello();

    public void Notify(string message) => 
        Debug.Log(message);
}

var builder = new ContainerBuilder();
builder.AddSingleton(typeof(DebugConsole));
builder.AddSingleton(typeof(Greater));
var container = builder.Build();
var console = container.Resolve<DebugConsole>();
console.Welcome(); // now it's work! Print "Hello, World!"!

It happens because enumerations is a lazy evaluation, and the container returns exactly the enumeration when it is resolved.

Code from Reflex Container

public IEnumerable<object> All(Type contract)
{
    return ResolversByContract.TryGetValue(contract, out var resolvers)
        ? resolvers.Select(resolver => resolver.Resolve(this))
        : Enumerable.Empty<object>();
}

So the previous code works, but the next one fails again:

Example 3. Not working.

public class DebugConsole
{
    private readonly Greater _greater;

    public DebugConsole(IEnumerable<Greater> greater) => 
        _greater = greater.First();
    
    public void Welcome() => 
        _greater.Hello();

    public void Notify(string message) => 
        Debug.Log(message);
}

This could be solved by explicitly calling the enumeration at creation time, for example, with .ToArray():

Added ToArray to Reflex Container

public IEnumerable<object> All(Type contract)
{
    return ResolversByContract.TryGetValue(contract, out var resolvers)
        ? resolvers.Select(resolver => resolver.Resolve(this)).ToArray()
        : Enumerable.Empty<object>();
}

I don't know if this is a bug or a feature. But in my opinion, it is a hack. Such functionality can be implemented by other means (if necessary), and leaving the enumeration as an enumeration. The containers I've worked with throw an exception here. And this seems to me to be the more expected behavior. What do you think about it?

[gustavopsantos]

Hello @Scrawach great finding here, actually the only reason im not calling .ToArray() for when resolving enumerations is to avoid allocating an array to fit all elements, not because I would like to support this hacky way of having circular dependencies.
So in current implementation, consumer can decide if he want to iterate using the returned IEnumerable with a foreach, or want to cache an array calling .ToArray() having now a contigous version of this enumerable to be able to iterate a bit faster.

[Scrawach]

Hello @gustavopsantos , good point, I agree. However, the problem is that at the moment when a consumer wants to cache an array by calling .ToArray() in the constructor, he won't be able to do it, because there are cyclic dependencies in the system. And he will find out about it exactly at the moment of caching, not at the moment of designing before that. And how painful it will be depends on the concrete situation and the time of the project's life. I guess, of course, architecture problems are not the concern of a DI container, but it would be convenient if the consumer found out about his problem much earlier.

Anyway, I've highlighted this point and if you think that performance is a more important factor here (as a killer feature), I certainly won't insist. Nevertheless, it might make sense to highlight this in the documentation in future.

[gustavopsantos]

@Scrawach Well, I believe most of people are not using much All<T> API, specially if performance is a concern, so I think in this case design consistency is more important than performance, thanks for pointing this out!
Im gonna be pushing this small change soon, thanks!

[Scrawach]

@gustavopsantos you're welcome! Thanks for the framework! :)

[gustavopsantos]

@Scrawach Added .ToArray() to .All() and to .All<T>() methods, ensuring all resolve kind APIs throw exception when resolving cyclic dependencies. Thank you so much for you detailed feedback ;)
https://github.com/gustavopsantos/Reflex/releases/tag/8.0.0