[discussion] bugfix - interface is needed for DI#236

[J0EK3R]

I’m currently trying to understand why the issue #236 with Autofac failing to resolve the dependency occurred. I’ve already found the crucial part of the code where the problem originates.

Registering the MouldKingDeviceManager by calling the Extension RegisterDeviceManager

internal class MouldKing : Vendor<MouldKing>
{
...
    protected override void Register(VendorBuilder<MouldKing> builder)
    {
...
        // device manager
        builder.RegisterDeviceManager<MouldKingDeviceManager>()
            .SingleInstance();
    }

The extension is registering the type as IBluetoothLEDeviceManager only not as TManager itself

public static class ContainerBuilderExtensions
{
    /// <summary>
    /// Register <typeparamref name="TManager"/> as implementation of <see cref="IBluetoothLEDeviceManager"/> in order to use it for device discovery
    /// </summary>
    /// <param name="builder">DI container builder</param>
    /// <returns>Registration builder in order to allow additional registration, such as .As<>()</returns>
    public static IRegistrationBuilder<TManager, ConcreteReflectionActivatorData, SingleRegistrationStyle> RegisterDeviceManager<TManager>(this ContainerBuilder builder)
        where TManager : class, IBluetoothLEDeviceManager
        => builder.RegisterType<TManager>().As<IBluetoothLEDeviceManager>().SingleInstance();

We could improve that by adding the .AsSelf() to the extension

    public static IRegistrationBuilder<TManager, ConcreteReflectionActivatorData, SingleRegistrationStyle> RegisterDeviceManager<TManager>(this ContainerBuilder builder)
        where TManager : class, IBluetoothLEDeviceManager
        => builder.RegisterType<TManager>().AsSelf().As<IBluetoothLEDeviceManager>().SingleInstance();

or the registration of the MouldKingDeviceManager:

        // device manager
        builder.RegisterDeviceManager<MouldKingDeviceManager>()
            .AsSelf()
            .SingleInstance();

then we could revert 5fd13f3

---

The other thing is why the UnitTests did not uncover the problem...

[J0EK3R]

These are improved unit tests that would have exposed the dependency‑resolution bug.

#239

[vicocz]

The other thing is why the UnitTests did not uncover the problem...
Wrong test setup, tested subject was impacted by setup itself which means it gave the wrong feeling of tested area.

Yet another thing you did not mention. Dev testing. Any contributor is expected to dev test his changes. Nobody's perfect, of course.
No blame, but it's not easy to code review all the contributions and there is nearly no chance to do some testing.

[J0EK3R]

Yet another thing you did not mention. Dev testing. Any contributor is expected to dev test his changes. Nobody's perfect, of course.
No blame, but it's not easy to code review all the contributions and there is nearly no chance to do some testing.

Yes, I fully agree - sorry :(

[J0EK3R]

Do you want to revert 5fd13f3 and remove IMouldKingDeviceManager?

[vicocz]

IMouldKingDeviceManager

I think we should force to use interfaces instead of registering such manager as the class itself (AsSelf()).
Generally said, we can add some registration method override:

    public IRegistrationBuilder<TManager, ConcreteReflectionActivatorData, SingleRegistrationStyle> RegisterDeviceManager<TInterface, TManager>()
        where TInterface : class
        where TManager : class, IBluetoothLEDeviceManager, TInterface
        => ContainerBuilder.RegisterDeviceManager<TManager>()
        .As<TInterface>().SingleInstance();

so as ithis registration can be simplified:

builder.RegisterDeviceManager<IMouldKingDeviceManager, MouldKingDeviceManager>();

this is IMHO acceptable, even though e.g., CaDADeviceManager requires yet another interface.