Avoid calling init method on each return method

Question

I'm implementing a module system into one of mine ASP.NET MVC applications and I am currently writing a module helper which all the modules can use to get some basic information about itself.

I'm using the namespace of the caller to identify the module and search it up in the database and retrieve the information.

Now I want to make everything static, so I don't have to instantiate it each time.

   public static class ModuleHelper
{
     private static  ModuleInstallations _module;

     static ModuleHelper()
     {
         Init();
     }

    private static void Init()
    {
         var stackTrace = new StackTrace();
         MethodBase method = stackTrace.GetFrame(2).GetMethod();
         var calledNamespace = method.ReflectedType.Assembly.GetName().Name;

        if (_module == null || calledNamespace != _module.Namespace)
        {
            LoadModuleInfo(calledNamespace);
        }
    }

    private static void LoadModuleInfo(string moduleNamespace)
    {
        var moduleService = new ModulesService();
        _module = moduleService.GetByNameSpace(moduleNamespace); 
    }

    public static string GetPathModule(this Controller controller, string pathFile)
    {
        Init();

        return string.Format("/Modules/{0}/{1}", _module.AreaName, pathFile);
    }

    public static string GetModuleAreaName(this Controller controller)
    {
        Init();

        return _module.AreaName;
    }

    public static int GetModuleId(this Controller controller)
    {
        Init();

        return _module.Id;
    }
}

The problem is that a previous module could get stored into the class and I have to check on each method if it's another module. If it's another module I have to retrieve the information of it.

What would be a good solution for this problem?

Answer 1

The first thing I don't like much is that you walk the callstack to find the caller. It's faster and more robust to find the assembly via a type that's part of that assembly. Since your methods are called on a controller, you could use controller.GetType().Assembly to find it.

The next issue is that you only store a single value and you do so in a non threadsafe fashion. I'd use a threadsafe dictionary that maps an assembly to the data instead.

Omitting details like argument validation, my code would look similar to the following:

public class ModuleInformation
{
    private static readonly ConditionalWeakTable<Assembly, ModuleInformation> _dict = new ConditionalWeakTable<Assembly, ModuleInformation>();

    private static ModuleInformation LoadInfo(Assembly assembly)
    {
        // replace with code to load the module information
        return new ModuleInformation();
    }

    public static ModuleInformation Get(Assembly assembly)
    {
        return _dict.GetValue(assembly, LoadInfo);
    }

    public string Id { get; private set; }
}

public static class ModuleInformationExtensions
{
    public static ModuleInformation GetModuleInformation(this Controller controller)
    {
        return ModuleInformation.Get(controller.GetType().Assembly);
    }

    public static string GetAreaName(this Controller controller)
    {
        return controller.GetModuleInformation().Id;
    }
}

Another dubious part of your code is:

var moduleService = new ModulesService();
_module = moduleService.GetByNameSpace(moduleNamespace);

Depending on the implementation of the ModulesService it might be a good idea to pass it in via dependency injection. Database access from static methods is usually a bad idea since the database should be injected. This is incompatible with your static design, so you might want to switch to instances which are then injected into the controllers.

Answer 2

Calling Init() for every method call is most likely a bad idea.

Your idiom is that you will need to Init() your data once at the first method call, and therafter you should not Init() it anymore, therefore you should do the following:

• Use a private boolean _isInit
• Only Init() once, then set _isInit to true
• On Init() calls, only proceed if _isInit is false.

In this specific use case this behavior seems to be even more important as obtaining the stack trace is most likely an expensive operation.

Answer 3

The problem is that a previous module could get stored into the class and i have to check on each method if it's another module. If it's another module i have to retrieve the information of it.

A static helper class should not be keeping state, as a general observation. I.E. set _module = null when you're done with it - every time.

What would be a good solution for this problem?

• keep a collection of ModuleInstallations.
• Add each object to the collection when it is fetched from the DB.
• Query the collection first, then go to the DB if it is not already in the collection.
• .NET provides memory caching classes.

Convenient, Easy, and Encapsulated

• ModuleInstallations should override object.Equals() (and by extension, GetHashCode)
  • Here is a GetHashCode implementation from stackoverflow

• Create a strongly typed collection, like List<ModuleInstallations>

• The collection should override object.Equals(object, object)

  • All this has to do is call ModuleInstallations.Equals override.

• Assuming a List<T>

  • now we can use Contains
  • Find(predicate) ... the predicate can be dynamic if need be
  • Lots of LINQ-y extension methods.

Answer 4

I would either refactor the class to be a ModuleFactory so it constructs and configures a new instance of a Module, and access its properties/methods outside, or refactor your init method to instantiate a new Module and return an instance of it to the methods.

Answer 5

I implemented ModuleHelper as a class (Module) that exists as an instance for each namespace loaded. And then made a ModuleHelper static class that creates the singletons for the calling namespace as needed. I used Unity to manage the singletons, but you can use your IoC of choice.

public class Module
{
    private ModuleInstallations Module { get; set; }
    public Module(string nameSpace)
    {
        Module = null; //get by nameSpace
    }
    public string GetPathModule(string pathFile)
    {
        return "";
    }
    public int GetModuleId()
    {
        return 0;
    }
    public string GetModuleAreaName()
    {
        return "";
    }
}
public static class ModuleHelper
{
    private static Lazy<IUnityContainer> defaultContainer = new Lazy<IUnityContainer>(() => new UnityContainer());
    private static IUnityContainer DefaultContainer
    {
        get { return defaultContainer.Value; }
    }
    [MethodImplAttribute(MethodImplOptions.NoInlining)]
    public static Module RegisterLoader(string nameSpace = null, IUnityContainer container = null)
    {
        if (nameSpace == null)
            nameSpace = Assembly.GetCallingAssembly().GetName().Name;
        if (container == null)
            container = DefaultContainer;
        Module loader = null;
        lock (container)
        {
            if (!container.IsRegistered<Module>(nameSpace))
            {
                loader = new Module(nameSpace);
                container.RegisterInstance<Module>(nameSpace, loader);
            }
            else
                loader = container.Resolve<Module>(nameSpace);
        }
        return loader;
    }
    [MethodImplAttribute(MethodImplOptions.NoInlining)]
    public static Module GetLoader(string nameSpace = null, IUnityContainer container = null)
    {
        if (nameSpace == null)
            nameSpace = Assembly.GetCallingAssembly().GetName().Name;
        if (container == null)
            container = DefaultContainer;
        var loader = container.Resolve<Module>(nameSpace);
        if (loader == null)
            loader = RegisterLoader(nameSpace, container);
        return loader;
    }
    [MethodImplAttribute(MethodImplOptions.NoInlining)]
    public static string GetPathModule(this Controller controller, string pathFile, string nameSpace = null, IUnityContainer container = null)
    {
        if (nameSpace == null)
            nameSpace = Assembly.GetCallingAssembly().GetName().Name;
        return GetLoader(nameSpace, container).GetPathModule(pathFile);
    }
    [MethodImplAttribute(MethodImplOptions.NoInlining)]
    public int GetModuleId(this Controller controller, string nameSpace = null, IUnityContainer container = null)
    {
        if (nameSpace == null)
            nameSpace = Assembly.GetCallingAssembly().GetName().Name;
        return GetLoader(nameSpace, container).GetModuleId();
    }
    [MethodImplAttribute(MethodImplOptions.NoInlining)]
    public string GetModuleAreaName(this Controller controller, string nameSpace = null, IUnityContainer container = null)
    {
        if (nameSpace == null)
            nameSpace = Assembly.GetCallingAssembly().GetName().Name;
        return GetLoader(nameSpace, container).GetModuleAreaName();
    }
}