Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

In my current .NET Core project, I want to inject two services into my controllers for dependency injection.

I wrote the following extension methods that each takes a IConfigurationSection, sets up the corresponding ldapContext and adds it to the services collection:

public class Startup
{
    public void ConfigureServices(IServiceCollection services)
    {
        var authenticationLdapConfiguration = Configuration.GetSection("AuthenticationLdap");
        services.ConfigureAuthenticationLdapServer(authenticationLdapConfiguration);
        var shibAttribLdapConfiguration = Configuration.GetSection("ShibbolethAttributesLdap");
        services.ConfigureShibbolethAttributeLdapServer(shibAttribLdapConfiguration);
    }
}

public static class ServiceCollectionExtensions
{
    public static void ConfigureAuthenticationLdapServer(this IServiceCollection services, IConfigurationSection authLdapConfiguration)
    {
        var ldapConnectionInfo = createLdapConnectionInfo(authLdapConfiguration);
        IAuthenticationLdapContext ldapContext = new AuthenticationLdapContext(ldapConnectionInfo);

        ldapContext.Bind();

        services.AddSingleton<IAuthenticationLdapContext>(ldapContext);
    }


    public static void ConfigureShibbolethAttributeLdapServer(this IServiceCollection services, IConfigurationSection shibAttribsLdapConfiguration)
    {
        var ldapConnectionInfo = createLdapConnectionInfo(shibAttribsLdapConfiguration);
        IShibbolethAttributeLdapContext ldapContext = new ShibbolethAttributeLdapContext(ldapConnectionInfo);

        ldapContext.Bind();

        services.AddSingleton<IShibbolethAttributeLdapContext>(ldapContext);
    }

    private static LdapConnectionInfo createLdapConnectionInfo(IConfigurationSection ldapServerConfiguration)
    {
        //set up connection info
    }
}

public interface IShibbolethAttributeLdapContext : ILdapContext {}

public interface IAuthenticationLdapContext : ILdapContext {}

public interface ILdapContext 
{
    //signatures of the functionality implemented below
    void Bind();
}

public class LdapContext : ILdapContext 
{
    public LdapContext(LdapConnectionInfo ldapConnectionInfo) { }
    //lots of functionality that I ultimately need
    public void Bind(){}
}

public class ShibbolethAttributeLdapContext : LdapContext, IShibbolethAttributeLdapContext
{
    public ShibbolethAttributeLdapContext(LdapConnectionInfo ldapConnectionInfo) : base(ldapConnectionInfo) {}
}

public class AuthenticationLdapContext : LdapContext, IAuthenticationLdapContext
{
    public AuthenticationLdapContext(LdapConnectionInfo ldapConnectionInfo) : base(ldapConnectionInfo) {}
}

I cannot change ILdapContext nor LdapContext as they are from an assembly that I don't manage.

Obviously, these methods only differ in the types that they wire up. Is there an elegant way to remove this duplication? My guess was to use generics or maybe a func, but I didn't come up with a solution.

UPDATE:
So I came up with this solution over the weekend that gets rid of the static extension methods as suggested by kayess:

public class Startup
{
    public IConfigurationRoot Configuration { get; }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        //set up services

        var authLdapContext = createBoundLdapContext<AuthenticationLdapContext>(Configuration.GetSection("AuthServer"));
        services.AddSingleton<IAuthenticationLdapContext>(authLdapContext);
        var shibbAttribLdapContext = createBoundLdapContext<ShibbolethAttributeLdapContext>(Configuration.GetSection("ShibAttribServer"));
        services.AddSingleton<IShibbolethAttributeLdapContext>(shibbAttribLdapContext);

    }

    private TContextInstance createBoundLdapContext<TContextInstance>(IConfigurationSection ldapServerConfiguration)
    where TContextInstance : LdapContext
    {
        TContextInstance ldapContext = createLdapContext<TContextInstance>(ldapServerConfiguration);

        ldapContext.Bind();

        return ldapContext;
    }

    private TContextInstance createLdapContext<TContextInstance>(IConfigurationSection ldapServerConfiguration) 
    where TContextInstance : LdapContext
    {
        var ldapConnectioninfo = createLdapConnectionInfo(ldapServerConfiguration);
        TContextInstance ldapContext = (TContextInstance)Activator.CreateInstance(typeof(TContextInstance), ldapConnectioninfo);

        return ldapContext;
    }

    private LdapConnectionInfo createLdapConnectionInfo(IConfigurationSection ldapServerConfiguration)
    {
        var ldapConnectionInfo = new LdapConnectionInfo();
        //set up ldapConnectionInfo
        return ldapConnectionInfo;
    }
}

Can I make it better?

share|improve this question
2  
As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. – Jamal Oct 21 '16 at 13:50
    
What is ShibbolethAttributeLdapContext? What is AuthenticationLdapContext? How are we supposed to know what base types or interfaces these classes inherit from? – BCdotWEB Oct 21 '16 at 15:23
    
Please correct me if I got this wrong, but why don't you just initialise and set up these services in your composition root? Making it static wont help you to make it testable, also if you want to organize them you could probably make a module (ninject lingo) of them and load them accordingly. – kayess Oct 21 '16 at 15:23
    
@kayess These are extension methods so that I can use them like services.ConfigureShibbolethAttributeLdapServer(ldapConnecti‌​on), as such, they have to be static. msdn.microsoft.com/en-us/library/bb383977.aspx – Thaoden Oct 21 '16 at 15:32
    
That's what I'm asking if these could be refactored to a non-static version.... – kayess Oct 21 '16 at 15:33

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.