2
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?

| improve this question | |
  • 2
    Why do you have Site implementing ILogin, if Site is not its own specific type of ILogin? To clarify with how I am interpreting your example, LoginStrategyA has a constructor that takes in some Site. With the current definition of Site, LoginStrategyA could have the constructor parameter type changed to ILogin and still allow Site to be passed in as a parameter. Is that the intent of Site? – eparham7861 Jul 28 at 2:07
  • @eparham7861 site contains ILogin functionality. Because there are many different types of sites, and ILogin functionality will be shared among them, it is abstracted out into its own class so that SiteA and SiteB could use ConcreteLoginA, and SiteC and SiteD could use ConcreteLoginB for example. The ConcreteLogins need an instance of a site in order to properly access state and any other methods that might be necessary, at least in this architecture. – hoidakalen Jul 28 at 2:25
  • So when SiteA is using ConcreteLoginA, if SiteA Login is called, does it always use the Login method specified by ConcreteLoginA, or are there instances where SiteA Login doesn't use ConcreteLoginA? – eparham7861 Jul 28 at 2:58
  • @eparham7861 it always uses the implementation passed to it. – hoidakalen Jul 28 at 3:23
5

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.

| improve this answer | |
  • 1
    Another way to break the circular dependency is to have the Site ctor accept a factory function instead of an ILoginStrategy; e.g. Site(Func<ISite, ILoginStrategy> loginStrategyFactory). Then you can do this.loginStrategy = loginStrategyFactory(site: this). The factory function would just call the LoginStrategyA constructor, passing along the provided site. If a DI container is used, some tweaking might be required, but it shouldn't be too much trouble. – Filip Milovanović Jul 28 at 22:11
  • 1
    @Filip: of course, but that kind of solution seems to be worth the extra effort only when Site needs to create more than one login strategy object, or when it requires to have precise control over the point in time when the login strategy is created. – Doc Brown Jul 29 at 6:21
  • Yes. It can also be used as a way to prevent/avoid partial construction of the strategy object (probably not important in this particular case, but generally speaking, that might be a part of the motivation). – Filip Milovanović Jul 29 at 14:16
1

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.

| improve this answer | |

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.