0

I have a class

public class CertificadoHandler : ICertHandler
{
    Type typeAfirmaValidate;

    public CertificadoHandler(){
        typeAfirmaValidate = typeof(AfirmaValidateCertificate);
    }

    //testing
    public void SetTypeAfirmaValidate(Type t)
    {
        typeAfirmaValidate = t;
    }

    public CredentialsData GetCredentialsData(X509Certificate cert)
    {
        return ((IValidateCertificate)Activator.CreateInstance(typeAfirmaValidate)).GetCredentialsData(cert);
    }
}

The reason of this code, is that I want to tell CertificadoHanlder which class have to instantiate very time GetCredentialsData gets called. How can i redesign this better. I don't like the fact that I am injecting a generic type (not a interface) but if I inject a interface i will not be able to instantiate it. How can i proceed?

Keypoints

  1. Certificadohandler is instantiated inside a singleton, so I want it to use new instances of Ivalidate certificate on each method call.(the app has concurrency so I want to avoid problems with sharing same variable between threads.

    1. Factory as parameter seems to be a better design

3 I want to unit test and mock IValidate certificate

0

3 Answers 3

1

Some options. Each has strengths and weaknesses.

Inject a transient instance

If you are using an IoC container, you can register a dependency as transient. For example, in Ninject you could use registerTransient. If you are using Unity, you can use TransienLifetimeManager. Then inject into your class per normal.

public void CompositionRoot(Container container)
{
    container.Register<IValidateCertificate, CertificateValidator>(new TransientLifetimeManager());
    container.RegisterType<ICerthandler, CertificadoHandler>(new ContainerControlledLifetimeManager());
}

public class CertificadoHandler : ICertHandler
{
    private readonly IValidateCertificate _validateCertificate;

    public CertificadoHandler(IValidateCertificate validateCertificate){
        _validateCertificate = validateCertificate;
    }

    public CredentialsData GetCredentialsData(X509Certificate cert)
    {
        return _validateCertificate.GetCredentialsData(cert);
    }
}

Inject a factory

You could inject a singleton factory, which will allow the class to instantiate whatever it needs, but still allow a unit test to override the injection. The advantage of using a factory (instead of a transient instance) is that the class can control when the instantiation happens and can even supply constructor arguments if they are needed.

Depending on your IoC framework, you may be able to avoid writing the factory and instead use an automatic factory.

public class ValidateCertificateFactory : IValidateCertificateFactory
{
    public IValidateCertificate GetInstance(string constructorArgument)
    {
        return new ValidateCertificate(constructorArgument);
    }
}

public void CompositionRoot(Container container)
{
    container.RegisterType<IValidateCertificateFactory, ValidateCertificateFactory>(new ContainerControlledLifetimeManager());
    container.RegisterType<ICerthandler, CertificadoHandler>(new ContainerControllerLifetimeManager());
}

public class CertificadoHandler : ICertHandler
{
    private readonly IValidateCertificateFactory _validateCertificateFactory;

    public CertificadoHandler(IValidateCertificateFactory validateCertificateFactory){
        _validateCertificateFactory = validateCertificateFactory;
    }

    public CredentialsData GetCredentialsData(X509Certificate cert)
    {
        return _validateCertificateFactory.Resolve("Foo").GetCredentialsData(cert);
    }
}

Inject a delegate

This is sort of like using a factory but avoids having to write a factory class.

public void CompositionRoot(Container container)
{
    container.RegisterType<Func<string, IValidateCertificate>>((arg) => new ValidateCertificate(arg));
    container.RegisterType<ICerthandler, CertificadoHandler>();
}

public class CertificadoHandler : ICertHandler
{
    private readonly Func<string,IValidateCertificate> _createValidator;

    public CertificadoHandler(Func<string,IValidateCertificate> func){
        _createValidator= func;
    }

    public CredentialsData GetCredentialsData(X509Certificate cert)
    {
        return _createValidator("Foo").GetCredentialsData(cert);
    }
}
5
  • The factory is the solution that fits best in my design/ requirements.
    – X.Otano
    Oct 6, 2017 at 23:54
  • The reason I need new instances on each call is to avoid problems with concurrent threads
    – X.Otano
    Oct 6, 2017 at 23:55
  • Ahhh... have you considered just using ordinary DI with instance-per-thread scope? For example, see AutoFac Thread Scope or Unity per thread lifetime management. That might be all you need.
    – John Wu
    Oct 7, 2017 at 0:29
  • I want to avoid IOC cause the code is not as complicated. What do you think about using a delegate factory as the injected parameter?
    – X.Otano
    Oct 7, 2017 at 7:39
  • Delegates are not as strongly typed as classes and can conceal dependencies. Best to inject interfaces, or factories with interfaces.
    – John Wu
    Oct 10, 2017 at 9:38
0

Like this?

    Func<IValidateCertificate> crearAfirmaValidate;

    public CertificadoHandler(){
        crearAfirmaValidate = () => new AfirmaValidateCertificate();
    }
    //testing
    public void SetTypeAfirmaValidate(Func<IValidateCertificate> factoria )
    {
        crearAfirmaValidate = factoria;
    }
4
  • I'd combine the setter and the constructor into one. public CertificadoHandler(Func<IValidateCertificate> factoria = null){ crearAfirmaValidate = factoria ?? () => new AfirmaValidateCertificate(); }
    – Caleth
    Oct 6, 2017 at 9:23
  • I dont want the callers to inject the dependency, only for testing purpose. Is that bad practice?
    – X.Otano
    Oct 6, 2017 at 9:32
  • That's what the default argument is for
    – Caleth
    Oct 6, 2017 at 9:32
  • What happens if I am using Activator for generating new instances?
    – X.Otano
    Oct 6, 2017 at 10:14
0

Your problem is that you are using an interface AND instantiating something concrete, even if you are using a System.Type object. No clue how you got to this design, but you need constructor dependency injection here:

public class CertificadoHandler : ICertHandler
{
    private IValidateCertificate validator;

    public CertificadoHandler(IValidateCertificate validator)
    {
        this.validator = validator ?? throw new ArgumentNullException(nameof(validator));
    }

    public CredentialsData GetCredentialsData(X509Certificate cert)
    {
        return validator.GetCredentialsData(cert);
    }
}

If you need a different instance each time, you either need a different instance of CertificadoHandler each time, or pass the IValidateCertificate object in as an argument. Given the very simple logic, it makes me question the need for this class at all.

If you actually need to validate a certificate against a number of validators, then inject a collection of them, then loop over them until you get a proper validation for the given certificate:

public class CertificadoHandler : ICertHandler
{
    private IEnumerable<IValidateCertificate> validators;

    public CertificadoHandler(IEnumerable<IValidateCertificate> validators)
    {
        this.validators = validators ?? throw new ArgumentNullException(nameof(validators));
    }

    public CredentialsData GetCredentialsData(X509Certificate cert)
    {
        foreach (var validator in validators)
        {
            var certValidation = validator.GetCredentialsData(cert);

            // Assuming GetCredentialsData returns null instead of throwing an exception
            // when the cert cannot be validated
            if (certValidation != null)
                return certValidation;
        }

        return null;
    }
}

And since you are dealing with an interface it allows you to easily mock these objects during unit tests.

1
  • There are some keypoints you need to know. I edited the question
    – X.Otano
    Oct 6, 2017 at 23:54

Your Answer

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

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