ServiceLocator.Current - bad implementation

Mar 17, 2009 at 7:23 PM
Hi guys,

thanks for this common interface....

One thing. I really think, this implementation is akward:
public static IServiceLocator Current
{
  get { return currentProvider(); }
}

At first, it does not throw a meaningful Exception, but just a NullReferenceException - which is evil by definition :-) At least there is not even a way to ask, whether a locator is available or not.

Some options that IMHO all would be better:
  1. Throw an proper exception if currentProvider is null + Provide a bool HasCurrent
  2. if currentProvider is null, return null
  3. or, even better, provide a NullServiceLocater, which throws ActivationExceptions on GetInstance-Method
What do you think?
Dec 11, 2009 at 8:14 AM

You are absolutely right about throwing a NullReferenceException. Throwing a NullReferenceException is always a bad thing for a reusable framework such as CSL. However, I don't think returning a NullServiceLocator is actually useful. An IoC container should be registered for users to be able to use CLS. Therefore, the NullServiceLocator should, when called, always throw an exception describing the fact that there is no container configured, and explain to the users what they should do to resolve this issue. This however would still be a bit more confusing than throwing such an exception directly after someone calls the Current property. There is no use of calling Current when there is no container registered, so there is no need for postponing communication with the caller.

Because using CSL without a configured IoC container is not useful, providing an HasCurrent property doesn't seem very useful to me. In normal cases you don't need to know whether there is a container or not, you simply need that container. In the exceptional situation where a developer needs to know this, the call to Current can be wrapped within try and catch statements.

Cheers

Dec 11, 2009 at 8:27 AM

I added a new work item to the issue tracker to fix this issue.

 

Dec 18, 2009 at 3:19 PM

I agree a more descriptive exception would be useful, though it's no deal-breaker for me personally.  I'm interested to understand the reason for "currentProvider" being a delegate that returns IServiceLocator, rather than an actual IServiceLocator reference, though?

Jan 2, 2010 at 1:21 PM

Why currentProvider is a delegate puzzled me for a while too, and I believe there is nothing you can do with the delegate that can't be done by supplying an interface, but there is at least one scenario where this leads to considerably less code. When registering an IServiceLocator implementation in a unit testing environment such as MSTest, you might want to register an IServiceLocator per thread (because MSTest runs tests on as many processors as it can). You can create a IServiceLocator decorator that implements the IServiceLocator and forwards all calls to a decorated (thread static) IServiceLocator, but because the CSL uses a delegate, you can also write this:

 

public static class ServiceLocatorUnitTestHelper
{
    [ThreadStatic]
    private static IServiceLocator locator;

    public static void Initialize()
    {
        ServiceLocator.SetLocatorProvider(() => locator);
    }

    public static void SetThreadLocalServiceLocator(
        IServiceLocator locator)
    {
        ServiceLocatorUnitTestHelper.locator = locator;
    }
}

And in your unit tests you can use this class as follows:

[AssemblyInitialize]
public static void AssemblyInitialize(TestContext context)
{
    ServiceLocatorUnitTestHelper.Initialize();
}

private ServiceLocatorImplementation serviceLocator =
new ServiceLocatorImplementation();

[TestInitialize]
public void TestInitialize()
{
IServiceLocator locator = this.serviceLocator;

ServiceLocatorUnitTestHelper.SetThreadLocalServiceLocator(locator);
}

[TestCleanup]
public void TestCleanup()
{
ServiceLocatorUnitTestHelper.SetThreadLocalServiceLocator(null);
}

[TestMethod]
public void Test()
{
// Arrange this.serviceLocator.RegisterSingle<IWeapon>(new Katana());

// Act
var warrior = new Samurai();

// Arrange
Assert.IsInstanceOf(warrior.Weapon, typeof(Katana));
}
Sep 23, 2014 at 8:41 PM
Edited Sep 23, 2014 at 8:48 PM
OS: Window 8.1
IDE: VS 2013 Express for Windows
Prism: StoreApps
IoC: Unity
Proj: Universal

Hi,

I'm having a problem getting this to work. Need help on how to initialize the Service, There's no protected override void ConfigureServiceLocator() in MvvmAppBase. Tried this during protected override Task OnInitializeAsync(IActivatedEventArgs args):
ServiceLocator.SetLocatorProvider(() => this._container.Resolve<IServiceLocator>());
and Current is still not set:
    public sealed partial class MainPage : VisualStateAwarePage
    {
        public MainPage()
        {
            this.InitializeComponent();

            NathsarTS.UILogic.Interfaces.IObservableService ObservableService = ServiceLocator.Current.GetInstance<NathsarTS.UILogic.Interfaces.IObservableService>();
        }
    }
...with no success. How do I get this to work in my environment?

Thanks!...
Sep 24, 2014 at 12:08 AM
Edited Sep 24, 2014 at 6:39 AM
Cancel that! After much searching and testing I finally got it after reading these posts on the Unity Adapter page.
..
private readonly IUnityContainer _container = new UnityContainer();
...
..
.
var locator = new UnityServiceLocator(_container);
ServiceLocator.SetLocatorProvider(() => locator);