IMetadatapublic interface IMetadata{    String ObjectFullName { get; set;  }    String PropertyName { get; set; }    String DisplayName { get; set; }    String DisplayFormat { get; set; }    Int32 DisplayOrder { get; set; }}public static class IMetadataExtensions{    public static String GetFullyQualifiedPropertyName(this IMetadata meta)    {        return String.Format({0}.{1}, meta.ObjectFullName, meta.PropertyName);    }}IMetadataContainer/// <summary>/// Contains metadata. Key will be either the PropertyName or the fully qualified PropertyName./// </summary>public interface IMetadataContainer{    IDictionary<String, IMetadata> Metadata { get; set; }}IMetadataProvider/// <summary>/// Contains a collection of all available metadata./// </summary>public interface IMetadataProvider{    IEnumerable<IMetadata> Metadata { get; set; }}public static class IMetadataProviderExtensions{    /// <summary>    /// Injects the supplied metadata into the container.    /// If there are conflicts, by default, the conflict will just replace the previous metadata information.    /// To prevent this, you may fully qualify your property names with the object's full name by setting    /// fullyQualifyOnConflict to 'true'.    /// </summary>    /// <returns>Where there any conflicts?</returns>    public static Boolean InjectMetadata(this IMetadataProvider provider, IMetadataContainer container, String objectFullName, Boolean fullyQualifyOnConflict = false)    {        // VALIDATE: Null checks.        if (container == null) { return true; }        if (provider == null || provider.Metadata == null) { return true; }        // EXECUTE: Iterate through all metadata grouped by their object adding them based on conflicts.        Boolean conflictsExist = false; // This part of the alg might have a logical error or two, I haven't tested yet, I wanted feedback on the design first.        IEnumerable<IMetadata> matchingMetadata = provider.Metadata.Where(m => m.ObjectFullName == objectFullName).GroupBy(m => m.ObjectFullName).Select(g => g.First());        foreach (IMetadata meta in matchingMetadata)        {            Boolean conflictsExistOnObject = matchingMetadata.Where(m => m.ObjectFullName == meta.ObjectFullName).Any(m => container.Metadata.ContainsKey(m.PropertyName));            conflictsExist = conflictsExist || conflictsExistOnObject;            if (conflictsExistOnObject && fullyQualifyOnConflict)            {                container.Metadata.Add(meta.GetFullyQualifiedPropertyName(), meta);            }             else if (conflictsExistOnObject && !fullyQualifyOnConflict)            {                container.Metadata.AddOrReplace(meta.PropertyName, meta);            }            else            {                container.Metadata.Add(meta.PropertyName, meta);            }        }        // RETURN: Whether ANY conflicts were detected.        return conflictsExist;    }}MetadataWarehouse/// <summary>/// A Singleton class that manages the applications Metadata by caching it/// instead of having to retrieve it from the database with every new HttpRequest./// </summary>public sealed class MetadataWarehouse : IMetadataProvider{    /// <summary>    /// A collection of all the Metadata in the database for this application.    /// </summary>    public IEnumerable<IMetadata> Metadata { get; set; }    private EntityModels db;    /// <summary>    /// Gets the MetadataWarehouse Singleton.    /// </summary>    public static MetadataWarehouse Get    {        get        {            if (_Get == null) { _Get = new MetadataWarehouse(); }            return _Get;        }    }    private static MetadataWarehouse _Get;    /// <summary>    /// Private Constructor since this is a Singleton.     /// If anyone can create this class then it's not truly a Singleton.    /// </summary>    private MetadataWarehouse()    {        this.db = new EntityModels();        _Refresh();    }    /// <summary>    /// Refreshes the Metadata from the database.    /// </summary>    public static void Refresh()    {        MetadataWarehouse.Get._Refresh();    }    private void _Refresh()    {        this.Metadata = this.db.Metadatas.ToList();    }}UsageMetadataWarehouse is created at the start of my MVC application and retrieves all of the metadata on first use. I made it a singleton to prevent having to grab the same exact metadata nearly every single HttpRequest. So now with each request I just callMetadataWarehouse.Get.InjectMetdata(container, Namespace.ObjectName, true)for each ObjectFullName that I need it for (which varies from 1-4 so far).Concerns:Is this a good use for the Singleton? Is there another way to achieve this without a Singleton in ASP.NET MVC?Is it good/bad design to use Extension Methods on Interfaces like I have for InjectMetadata?The only thing I'm not budging on is my usage of sectional comments (such as // VALIDATE: Check nulls). I find they help me improve readability of my code (directly and indirectly) and increase readability speed? (allows me and others to skim the code quicker)
Persisting data between HttpRequests in MVC
c#;singleton;interface
Not a huge amount of code to review, just a few things:Get is a particular bad name for for the singleton property as Get typically implies an action rather than a property and such is expected to be a method. Traditionally static singleton instances are named Instance.The singleton instance is not thread-safe. Don't know about the rest of the code and how the startup works but it's possible for multiple requests coming in and obtaining two different instances. Even if it's currently not possible (because the instance is being accessed by the startup code from a single thread) you should still use Lazy<T> or lock it so you won't have to worry about it even if the code changes.The public static Refresh is sub-optimal since it's not clear what exactly it is refreshing - it's hiding the fact that it's refreshing the data of the singleton. I would get rid of it and just make the _Refresh method public (after renaming it to Refresh). It won't create much more typing work but be a lot more explicit about what exactly it is operating on.The main problem with static singleton instances is that they are prone to be accessed from everywhere - this creates implicit, invisible dependencies which can come easily back and bite you when it comes to unit testing and refactoring. So as much as possible classes which use it should be given an IMetadataProvider as a parameter to make it visible what they depend on.You should consider the use of an IoC container (Windsor, Ninject, Unity, ...) which greatly helps with this. Don't know out of the top of my head how it works in ASP.NET MVS but I'm sure it has support for resolving dependencies of controllers via IoC container.For extension methods it's customary to throw a ArgumentNullException when the this parameter is null. This brings it more in line with normal objects which would throw a NullReferenceException when they are null while being dereferenced.The GroupBy in here seems somewhat pointless:provider.Metadata.Where(m => m.ObjectFullName == objectFullName)                 .GroupBy(m => m.ObjectFullName).Select(g => g.First());The Where already gives you all objects with matching objectFullName so you will only have one resulting group. You may as well take the first element after the Where and not bother grouping.