Service Locator C#

Question

I made this service locator to be used on my game projects, but it uses pure c#. Basically, new services needs to inherit from the common IService interface to be registered. I also added events to provide the services so that you cannot run into initialization timing null errors, and the dictionaries use string as keys to use less memory than System.Type

using System;
using System.Collections.Generic;
namespace Game.ServiceLocator
{
 public static class GameServiceLocator
 {
 private static readonly Dictionary<string, IService> _gameServices = new Dictionary<string, IService>();
 private static readonly Dictionary<string, Delegate> _serviceAvailabilityCallbacks = new Dictionary<string, Delegate>();
 public static void RegisterGameService<T>(T service) where T : IService
 {
 string serviceName = typeof(T).Name;
 if (_gameServices.ContainsKey(serviceName))
 {
 _gameServices[serviceName] = service;
 }
 else
 {
 _gameServices.Add(serviceName, service);
 }
 NotifyUsersAboutServiceAvailability(serviceName, service);
 }
 public static void DeRegisterGameService<T>() where T : IService
 {
 string serviceName = typeof(T).Name;
 if (_gameServices.ContainsKey(serviceName))
 {
 _gameServices.Remove(serviceName);
 }
 }
 public static void GetGameServiceWhenAvailable<T>(Action<IService> listener) where T : IService
 {
 string serviceName = typeof(T).Name;
 AddServiceAvailabilityCallbackListener(serviceName, listener);
 if (_gameServices.TryGetValue(serviceName, out IService service))
 {
 listener.Invoke(service);
 return;
 }
 }
 public static void RemoveServiceListenerCallback<T>(Action<IService> listener) where T : IService
 {
 string serviceName = typeof(T).Name;
 if (_serviceAvailabilityCallbacks.TryGetValue(serviceName, out Delegate value))
 {
 _serviceAvailabilityCallbacks[serviceName] = Delegate.Remove(value, listener);
 return;
 }
 }
 private static void AddServiceAvailabilityCallbackListener(string serviceName, Action<IService> listener)
 {
 if (_serviceAvailabilityCallbacks.TryGetValue(serviceName, out Delegate value))
 {
 _serviceAvailabilityCallbacks[serviceName] = Delegate.Combine(value, listener);
 return;
 }
 _serviceAvailabilityCallbacks.Add(serviceName, listener);
 }
 private static void NotifyUsersAboutServiceAvailability(string serviceName, IService service)
 {
 if (_serviceAvailabilityCallbacks.TryGetValue(serviceName, out Delegate serviceAvailabilityCallback))
 {
 serviceAvailabilityCallback?.DynamicInvoke(service);
 }
 }
 }
}
//this is in another file
namespace Game.ServiceLocator
{
 public interface IService
 {
 }
}

Answer 1

Pattern or Anti-pattern

Generally speaking Dependency Injection is preferred over Service Locator. SL is considered anti-pattern sometimes because it makes unit testing harder and tightly couples all your classes to itself.

You can read a lot about this topic over the internet:

• https://freecontent.manning.com/the-service-locator-anti-pattern/
• https://deviq.com/antipatterns/service-locator
• https://www.jimmybogard.com/service-locator-is-not-an-anti-pattern/
• etc.

My main point here, use Service Locator as a conscious decision: Among the alternatives this solution suits the best for your needs.

_gameServices

Many threads could use the GameServiceLocator class concurrently. It is advisable to use concurrent collections like ConcurrentDictionary to make your modification actions (add and remove) atomic.

For example two threads might call the RegisterGameService method at the same time with the same type. It might happen that both receives false for the ContainsKey and tries to Add the new type to the underlying dictionary. The second Add call will end up with a ArgumentException stating that the key already exist. This is due to the fact that the existency check and the modification action are not a single operation.

You might have the temptation to use TryAdd as the safe method alternative to the Add. It won't throw exception just returns false if the key already exist.

BUT the Dictionary itself has limited thread safety guarantees:

A Dictionary<TKey,TValue> can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

For thread-safe alternatives, see the ConcurrentDictionary<TKey,TValue> class or ImmutableDictionary<TKey,TValue> class.

Parameter validation

I would suggest to do perform input parameter validation at least null checks before you do anything with the parameter.

RegisterGameService

IMHO making the method(s) generic just to capture the type name feels a bit clumsy. You can retrieve that information without the type parameter.

Here is an example:

using System;
using System.Collections.Generic;
using System.Collections.Concurrent;
using System.Collections.Immutable;
 
public class Program
{
 public static void Main()
 {
 Print(new [] { "A"});
 Print(new List<string> { "A"});
 Print(new ConcurrentBag<string> { "A" });
 Print(ImmutableArray.Create<string>("A"));
 }
 
 public static void Print(IEnumerable<string> s)
 => Console.WriteLine(s.GetType().Name);
}

Output

String[]
List`1
ConcurrentBag`1
ImmutableArray`1

So, I would suggest to make your API simpler by not using generics:

public static void RegisterGameService(IService service) 

Also please prefer FullName over Name to avoid name collision (same type name under different namespaces).

DeRegisterGameService

I'm not a native speaker but I would suggest to use Deregister over DeRegister. Also suffixing with GameService all the time does not provide extra information. It might suggest that this utility class may or may not have other deregister functions for other entities than game services.

GetGameServiceWhenAvailable

This name is a bit misleading since you don't get anything back (the return type is void).

Also without knowing why don't you have a simple Get method and why do you need to provide a delegate to execute certain logic against a game service this API design seems a bit overkill for me. There might be a fair reason but without knowing that it just complicates your code and the its usage unnecessary.

The blank return; statement unnecessary as well. Here and inside RemoveServiceListenerCallback methods.

AddServiceAvailabilityCallbackListener

I haven't used Delegate.Combine so, I'm not sure what would happen if you register multiple callbacks for the same service by calling the AddServiceAvailabilityCallbackListener several times with the same serviceName parameter.