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

SimpleContainer.GetAllInstances only return one instance? #579

Closed
CaulyKan opened this issue Jan 21, 2019 · 2 comments
Closed

SimpleContainer.GetAllInstances only return one instance? #579

CaulyKan opened this issue Jan 21, 2019 · 2 comments

Comments

@CaulyKan
Copy link
Contributor

I'm not sure is this a bug or a feature? Or am I using GetAllInstances in a wrong way?

    class Program
    {
        static void Main(string[] args)
        {
            var container = new SimpleContainer();
            container.RegisterInstance(typeof(ITest), "A", new TestA());
            container.RegisterInstance(typeof(ITest), "B", new TestB());
            var TestA = container.GetInstance<ITest>("A");      // OK
            var TestB = container.GetInstance<ITest>("B");      // OK
            var results = container.GetAllInstances<ITest>();   // Only TestA, expect to have both
        }

        public interface ITest { }

        public class TestA : ITest { }
        public class TestB : ITest { }
    }
@CaulyKan
Copy link
Contributor Author

After looking into the source I found that the correct way to GetAllInstances is to make your instances having the same key:

        [Fact]
        public void Instances_registered_with_different_keys_get_all_instances_return_all()
        {
            var container = new SimpleContainer();
            container.Instance<object>(new object());
            container.RegisterInstance(typeof(object), null, new object());

            var results = container.GetAllInstances<object>();

            Assert.Equal(2, results.Count());
        }

But however, The current GetAllInstances can only get instances that have null as key. I think that a little more work does make sense:

  1. Make it clear in xml document.
  2. Provide a GetAllInstances<Type>(string key = null), which not only can get instances with specified key, but also gives a hint to user that instances are grouped within a key.

CaulyKan added a commit to CaulyKan/Caliburn.Micro that referenced this issue Jan 21, 2019
nigel-sampson added a commit that referenced this issue Jan 21, 2019
#579 Add optional key to GetAllInstances
@nigel-sampson
Copy link
Contributor

Thanks for finding this, I've merged #580.

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

2 participants