Circular dependency with dependency injection

Question

public interface ILogin
{
    Task Login();    
}

public class LoginStrategyA : ILogin
{
    Site site;

    public LoginStrategyA(Site site) => this.site = site;
    
    public Task Login()
    {
        
    }
}

public class Site : ILogin
{
    ILogin loginStrategy;   

    public Site(ILogin loginStrategy) => this.loginStrategy = loginStrategy;

    public Task Login() => loginStrategy.Login();
}

I would like to abstract core pieces of functionality into separate classes. In the example above, the Login functionality is abstracted out to an ILogin implementation. This ILogin implementation will need to access Site members. It seems redundant and bizarre to define a separate ILogin that will take a Site as a parameter. On the other hand, I am not sure how to handle the instantiation of these classes in a way that allows for dependency injection. Is there a better way to approach these types of situations?

Answer 1

Naming

ILogin is an abstract interface for different login strategies, so better call it accordingly

  public interface ILoginStrategy
  {
       Task Login();    
  }

Inheritance

By better naming, the correct usage of inheritance becomes clearer: a Site has a login strategy, but it is not a login strategy by itself. So don't derive Site from ILoginStrategy, even if it has a member Login with the same signature. That is only a coincidence, Site objects are obviously not intended to be used as ILoginStrategys by itself.

Dependency injection

To be able to inject a mock Site into a concrete ILoginStrategy, make an abstract site interface, and derive Site from it:

public class Site : ISite
{
}

Put all Site methods into ISite which are required by login strategy objects.

Now implement LoginStrategyA in terms of ISite:

public class LoginStrategyA : ILoginStrategy
{
    ISite site;
    public LoginStrategyA(ISite site) => this.site = site;
    // ...

Resolving cyclic dependencies between objects

If a Site constructor requires an ILoginStrategy, and LoginStrategyA constructor requires an ISite, it is not clear which object to construct first. As a solution, either allow a Site object to exist temporarily without an ILoginStrategy, and add a method to set or change this strategy afterwards, or vice versa:

     var site = new Site(null);  
     // alternatively:  site = new Site(new EmptyLoginStrategy());

     var loginStrategy = new LoginStrategyA(site);
     site.SetLoginStrategy(loginStrategy);

That may look a little bit ugly, but when objects have cyclic dependencies, there is seldom a better way around it.

Answer 2

One possibility is to create a third class:

class SiteLogin {
}

class LoginStrategyA : ILogin {
   public LoginStrategyA(SiteLogin siteLogin);
}

class Site {
   public Site(SiteLogin siteLogin, ILogin login);
}

Everything that LoginStrategyA needs to access should be extracted to SiteLogin. That way both Site and LoginStrategyA can access it without creating a circular reference.

Another possibility, just pass Site as a parameter don't inject it:

class LoginStrategyA : ILogin {
   public void Login(Site site)
}

You don't need to inject everything, often times passing instances as method parameters is an simpler and better approach.