Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong constructor called #22

Open
mbp opened this issue Nov 22, 2017 · 4 comments
Open

Wrong constructor called #22

mbp opened this issue Nov 22, 2017 · 4 comments

Comments

@mbp
Copy link
Contributor

mbp commented Nov 22, 2017

Consider the following code

public interface IMyService
{
}

public class MyService : IMyService
{
    public MyService()
    {
        Console.WriteLine("Default constructor called!");
    }

    public MyService(params string[] foo)
    {
        Console.WriteLine("Custom constructor called!");
    }
}

I register it as follows:

container.Register(Component.For<IMyService>().ImplementedBy<MyService>());

When I call container.Resolve<IMyService>() from a normal console application not involving castle-windsor-ms-adapter, the default constructor is called.

When I call container.Resolve<IMyService>() from an ASP. NET Core project using this library (return WindsorRegistrationHelper.CreateServiceProvider(container, services); in Startup.cs, the second constructor is called.

I would have expected the default constructor always be called, as by out-of-the-box Castle Windsor behaviour. Am I missing anything?

@hikalkan
Copy link
Member

This library does nothing about the constructor selection. So, I could not understand why there is a difference here.

However, I believe the constructor design of this sample is very tricky and badly designed in general. You should remove the first constructor. In addition, if you use dependency injection, it will be a problem to pass string[] to the second ctor. So, I think this is not a good and realistic sample to spend time to find the problem :)

@acjh
Copy link

acjh commented Nov 23, 2017

How constructor is selected by Windsor:

  • It will look at the component's constructors and see which of them it can satisfy (for which of them it can provide all required parameters).
  • It will then see how many parameters each satisfiable constructor has, and pick the one with most parameters (the greediest one).

If you want the default constructor, you can do this:

using Castle.Core;

public class MyService : IMyService
{
    public MyService()
    {
        Console.WriteLine("Default constructor called!");
    }

    [DoNotSelect]
    public MyService(params string[] foo)
    {
        Console.WriteLine("Custom constructor called!");
    }
}

@mbp
Copy link
Contributor Author

mbp commented Nov 23, 2017

@hikalkan @acjh

I provided a simplified example. The service in my real world scenario is unfortunately from a separate library where I am not able to add [DoNotSelect] or even modify the library design.

@acjh

It does not matter how much you quote from the documentation; I provided a use case where it would not follow these rules across applications. I don't see any reason why Castle.Windsor in combination with castle-windsor-ms-adapter would suddenly think it can resolve params string[] foo, because surely I cannot do container.Resolve<string>(). By the way, you can replace string with any type, even custom type, and you get the same results.

I will likely have to find a different way to solve it, for example registering the component using a factory method, or not using dependency injection for this dependency. But I wanted to report this issue since I felt it was important for others to know that your application might break or provide non-deterministic behaviour when e.g. moving from ASP.NET to ASP.NET Core. Obviously I am now already a bit worried what other types of things it might resolve differently during the runtime of my application.

@hikalkan
Copy link
Member

Thank you @mbp
As I said, this library does not interfere to constructor selection, it's a deeper thing inside Windsor. However, assuming you tested correctly, it seems the behaviour changes.
I think it's a edge case, but I will check and think on that when I have more time.
Thank you for reporting that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants