TryGetInstance - should this fail?

[Jimmy Bogard]

[TestFixture]

public class TryGetInstanceWithPluggedType

{

public interface IFoo {}

public class Foo : IFoo {}

[Test]

public void Should_return_same_instance_with_try_get_instance_as_get_instance()

{

var container = new Container(cfg => cfg.For<IFoo>().Use<Foo>());

container.GetInstance<Foo>().ShouldNotBeNull();

container.TryGetInstance<Foo>().ShouldNotBeNull();

}

}

This is failing right now.  Am I misunderstanding the intention of "TryGetInstance"?

Thanks,

Your friendly neighbor

[Brian Chiasson]

That looks like a bug to me. The test passes if you change from the concrete type to the plugged type - container.TryGetInstance<IFoo>().

--
You received this message because you are subscribed to the Google Groups "structuremap-users" group.
To post to this group, send email to structure...@googlegroups.com.
To unsubscribe from this group, send email to structuremap-us...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/structuremap-users?hl=en.

[Corey Sullivan]

To me the calls of GetInstance<T> and TryGetInstance<T> hold different
meanings when creating concrete instances.

TryGetInstance<T> should only create an instance if the container has
a configured instance for that concrete type. If TryGetInstance<T>()
is called and a default instance is configured, it will return the
default instance for that type; else, it will return null. This holds
true when TryGetInstance<T>(string) is called but for named instances.
TryGetInstance<T> will look for only configured instances in the
container.

GetInstance<T> will create a type even if the container doesn't have a
configured instance for that concrete type.

container.TryGetInstance<Foo>().ShouldNotBeNull() should always be
false if you do not configure an (default) instance for that plugin
family.

Also for the above examples, container.For<IFoo>().Use<Foo>() doesn't
mean anything in this situation. Only container.For<Foo>()... would
have any meaning for TryGetInstance<Foo> calls.

[Jimmy Bogard]

Maybe the method name needs to change then?  I understand your description of how the implementations are different.  But the name implies functionality similar to the TryParse pattern.  If I have two methods "Foo" and "TryFoo", the implication is that the latter has the exact same functionality of the former, except that exceptions would never be thrown.

If you call TryGetInstance<Foo>(Profile.Default), it returns the correct instance.  Which is what led me to think that this might be a bug, that the codepath for supplying the profile name was much different than not supplying one.

[Corey Sullivan]

TryGetInstance<Foo>(string) works because of a bug. That overload
calls _pipelineGraph.HasInstance(pluginType, instanceKey) which calls
FindInstance on the pluginType's IInstanceFactory (InstanceFactory is
the concrete.) InstanceFactory.FindInstance has a bug where the method
calls _instances[name] and coalesces it with
InstanceFactory.MissingInstance. MissingInstance is initialized with
new ConfiguredInstance(typeof(Foo)) and will return an instance of
Foo. _instance[name] will not fail even if the supplied instanceKey
does not exist in the _instances cache.

Cache<,>[string] has a side-effect where it will add the supplied
instanceKey to the cache if it is not already contained in the cache.
The problem is that the _onMissing(key) delegate will return null by
default. Therefore, _instances will contain an entry that is [DEFAULT,
null] and when FindInstance coalesces that with
InstanceFactory.MissingInstance it will return MissingInstance, but
MissingInstance will not be in the _instances cache. Another side
effect of this call is that WhatDoIHave() or any other call that
relies on Foo's InstanceFactory will fail due to the cache have
[DEFAULT, null].

I agree that TryGetInstance does not have same semantic as TryParse,
but GetInstance doesn't do exactly what I would expect either by that
convention. GetInstance<Foo>() should not return a Foo instance if we
go off the convention that it needs to be configured in the container;
it should throw an exception. The intended implementation is that
GetInstance works in the same vein as the auto-wiring creation.
Creating a default instance of a concrete (and only failing if its
dependency tree can't be constructed) whether it is created with
GetInstance or through dependency injection seems to be the intended
implementation of the container.

On Oct 5, 9:33 pm, Jimmy Bogard <jimmy.bog...@gmail.com> wrote:
> Maybe the method name needs to change then?  I understand your description
> of how the implementations are different.  But the name implies
> functionality similar to the TryParse pattern.  If I have two methods "Foo"
> and "TryFoo", the implication is that the latter has the exact same
> functionality of the former, except that exceptions would never be thrown.
>
> If you call TryGetInstance<Foo>(Profile.Default), it returns the correct
> instance.  Which is what led me to think that this might be a bug, that the
> codepath for supplying the profile name was much different than not
> supplying one.
>

> > structuremap-us...@googlegroups.com<structuremap-users%2Bunsu...@googlegroups.com>
> > .