ReadOnlyDictionary Template

A re-usable template for a read only dictionary.
Edit: I changed Keys and Values collection to IEnumerable. Previously it was ICollection which has a Remove and Add method.

public interface IReadOnlyDictionary<TKey, TValue>
    {
        bool ContainsKey(TKey key);

        IEnumerable<TKey> Keys { get; }
        
        IEnumerable<TValue> Values { get; }
        
        int Count { get; }
        
        bool IsReadOnly { get; }
        
        bool TryGetValue(TKey key, out TValue value);
        
        TValue this[TKey key] { get; }
        
        bool Contains(KeyValuePair<TKey, TValue> item);
        
        void CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex);
        
        IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator();
    }

    public class ReadOnlyDictionary<TKey, TValue> : IDictionary<TKey, TValue>, IReadOnlyDictionary<TKey, TValue>
    {
        private readonly IDictionary<TKey, TValue> dictionary;

        public ReadOnlyDictionary()
        {
            this.dictionary = new Dictionary<TKey, TValue>();
        }

        public ReadOnlyDictionary(IDictionary<TKey, TValue> dictionary)
        {
            this.dictionary = dictionary;
        }

        public void Add(TKey key, TValue value)
        {
            throw new NotSupportedException("This dictionary is read-only");
        }

        public bool ContainsKey(TKey key)
        {
            return this.dictionary.ContainsKey(key);
        }

        public ICollection<TKey> Keys
        {
            get
            {
                return this.dictionary.Keys;
            }
        }

        public bool Remove(TKey key)
        {
            throw new NotSupportedException("This dictionary is read-only");
        }

        public bool TryGetValue(TKey key, out TValue value)
        {
            return this.dictionary.TryGetValue(key, out value);
        }

        public ICollection<TValue> Values
        {
            get
            {
                return this.dictionary.Values;
            }
        }

        public TValue this[TKey key]
        {
            get
            {
                return this.dictionary[key];
            }
            set
            {
                throw new NotSupportedException("This dictionary is read-only");
            }
        }

        public void Add(KeyValuePair<TKey, TValue> item)
        {
            throw new NotSupportedException("This dictionary is read-only");
        }

        public void Clear()
        {
            throw new NotSupportedException("This dictionary is read-only");
        }

        public bool Contains(KeyValuePair<TKey, TValue> item)
        {
            return this.dictionary.Contains(item);
        }

        public void CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex)
        {
            this.dictionary.CopyTo(array, arrayIndex);
        }

        public int Count
        {
            get
            {
                return this.dictionary.Count;
            }
        }

        public bool IsReadOnly
        {
            get
            {
                return true;
            }
        }

        public bool Remove(KeyValuePair<TKey, TValue> item)
        {
            throw new NotSupportedException("This dictionary is read-only");
        }

        public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()
        {
            return this.dictionary.GetEnumerator();
        }

        System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
        {
            return (this.dictionary as System.Collections.IEnumerable).GetEnumerator();
        }

    }

Using the Factory Method Pattern

The factory method pattern is a creational software design pattern that encapsulates how to construct a dependency into method.  The method could return any number of concrete implementations. However the consuming code only accesses the dependency through an interface or abstract base class.  This pattern is useful when there is distinct logic to return one concrete class over another.  It is also an effective pattern when working with legacy code, as it is easier to introduce a new method than a whole new interface.

The essence of the pattern is simply a virtual method that contains logic to create and/or return an instance of a dependency used by the consuming class.

Here's an example using the pattern to allow for unit testing substitution.

namespace FactoryMethodDemo
{
    public class ProductMaintenance
    {
        public void EditProduct(int id, string name, decimal price)
        {
            var dataAccess = this.CreateDataAccess();
            var product = dataAccess.GetProductById(id);
            product.Name = name;
            product.Price = price;
            dataAccess.Save(product);
        }

        public virtual DataAccessLayer CreateDataAccess()
        {
            return new DataAccessLayer();
        }
    }

    public class DataAccessLayer 
    {
        public virtual Product GetProductById(int id)
        {
            // Expensive database call...
            // Omitted
            throw new NotImplementedException();
        }

        public virtual void Save(Product product)
        {
            // Omitted
            throw new NotImplementedException();
        }

    }

    public class Product
    {
        public string Name { get; set; }
        public int Id { get; set; }
        public decimal Price { get; set; }

    }

}

In this example the CreateDataAccess method is the factory method pattern. Granted this is a very simple example, the method could contain a great deal of initialisation logic, or logic to determine which DataAccessLayer to return if there is more than one data store.  This could be useful when the database is partitioned (sharded) across multiple servers.

Things to note:

• Always make factory methods virtual. This always for testing as well as other variations in other sub-classes.
• Prefer to name the method with clear indicators about how it behaves.  I have chosen the word "Create" to inform consumers that each call to the method will create a new DataAccessLayer instance.
• Prefer to make the return type an interface. This may not be feasible with legacy code, hence my demo here uses virtual methods. This is easier to introduce into an existing code base than an interface. Although if its possible introduce an interface.

Here's the test code to test the ProductMaintenance class.

namespace TestProject1

{
    using FactoryMethodDemo;
    using Microsoft.VisualStudio.TestTools.UnitTesting;
    using Rhino.Mocks;

    public class ProductMaintenanceTestHarness : ProductMaintenance
    {
        private DataAccessLayer mock;

        public ProductMaintenanceTestHarness(DataAccessLayer mockDataAccessLayer)
        {
            this.mock = mockDataAccessLayer;
        }

        public override DataAccessLayer CreateDataAccess()
        {
            return this.mock;
        }
    }

    [TestClass]
    public class ProductMaintenanceTest
    {
        public TestContext TestContext { get; set; }

        [TestMethod]
        public void EditProductTest()
        {
            var productTestData = new Product
            {
                Id = 1,
                Name = "Bar",
                Price = 89.95M,
            };

            var mockDataAccessLayer = MockRepository.GenerateMock<DataAccessLayer>();  // Able to mock because methods are virtual
            mockDataAccessLayer.Expect(m => m.GetProductById(1)).Return(productTestData);
            mockDataAccessLayer.Expect(m => m.Save(productTestData));

            var subject = new ProductMaintenanceTestHarness(mockDataAccessLayer);
            subject.EditProduct(1, "Foo", 99.99M);

            mockDataAccessLayer.VerifyAllExpectations();
            Assert.AreEqual(99.99M, productTestData.Price);
        }
    }

}

Using a singleton (anti?)pattern

Some things that always apply to a Singleton Pattern (a pattern that falls into the creational pattern category):

• Think twice before using a singleton. Singletons can be bad for: parallel unit tests, dependency coupling, scalability, performance, memory management, complex threading issues (see here and here and countless others).
• Always use interfaces with your singletons.
• Always return the interface from the default static property.
• Prefer using a static default property that returns an interface over static methods that access a private static field. (This helps enforce the pattern when other developers add new members to the singleton).
• Always use a private constructor, this prevents any ad-hoc use of the class bypassing the singleton. (No one can create the class so you can only use the static Default property).
• Seal your singletons.  This will disallow someone sub-classing it then instantiating it as a transient class and bypassing the singleton.
• By definition, a singleton should be constructing only when it is first used.

Ok, so you insist on using a Singleton. Very well, lets try to do it with as least smelly code as possible...

All code examples make use of this interface.

namespace SingletonDemo
{
    public interface ISomeSingleton
    {
        void DoSomething();
        string Stuff { get; }
    } 

}


Use a Simple Singleton as a first option. Only use this when you know there will be no thread contention at initialisation time of the singleton. Or maybe it doesn't matter if there is.  This works well for unit testing because you can easily inject a mock singleton using a private accessor (reflection) or you could add a internal setter to the Default property and allow your test project access to internals (see this blog post for more details).

namespace SingletonDemo
{
    public sealed class SimpleSingleton : ISomeSingleton
    {
        private static ISomeSingleton defaultInstance;

        private SimpleSingleton()
        {
            // private ctor to prevent transient usage.
        }

        public static ISomeSingleton Default
        {
            get
            {
                return defaultInstance ?? (defaultInstance = new SimpleSingleton());
            }
        }

        public void DoSomething()
        {
            // omitted...
        }

        public string Stuff
        {
            get
            {
                // omitted...
                return string.Empty;
            }
        }
    }
}

Use what I call a ContentionSingleton when you know there will be thread contention when the singleton is first accessed. This example guarantees only one thread can create the singleton, all other threads wait until it is initialised.

namespace SingletonDemo
{
    public sealed class ContentionSingleton : ISomeSingleton
{
        private static readonly object SyncRoot = new object();
        private static volatile ISomeSingleton defaultInstance;

        private ContentionSingleton()
        {
            // Intialisation code...
        }

        public ISomeSingleton Default
        {
            get
            {
                if (defaultInstance == null)
                {
                    lock (SyncRoot)
                    {
                        if (defaultInstance == null)
                        {
                            defaultInstance = new ContentionSingleton();
                        }
                    }
                }

                return defaultInstance;
            }
        }

        public void DoSomething()
        {
            // Omitted...
        }

        public string Stuff
        {
            get
            {
                // Omitted...
                return string.Empty;
            }
        }
    }
}

Things to note:

• It is best practise to use a dedicated and separate lock object.  Never use the same lock object for different purposes. The lock object should always be private; no one outside this class should be using it.
• Declare the singleton instance as "volatile".  This indicates to the runtime that this object might be changed by different threads at the same time. This exempts this field from compiler optimisation that assumes only one thread will access this object at a time. It also guarantees the most up to date value will be in the field at all times.
• Private constructor.
• Double check the field both outside the lock and in. This ensures that 2 (or more) threads trying to initialise the singleton at the same time one enters the lock and one waits. Then when the first exits the lock the second will enter the lock, when it does it must not re-initialise the singleton.
• This still works well with unit testing for the same reasons as SimpleSingleton above.

There is another way to create singleton behaviour which I am not a fan of, Type Initialised Singleton:

namespace SingletonDemo
{
    public sealed class TypeInitialisedSingleton : ISomeSingleton

{
        private static readonly ISomeSingleton DefaultInstance;

        static TypeInitialisedSingleton()
        {
// .NET Runtime guarantees to run static constructors exactly once.
DefaultInstance = new TypeInitialisedSingleton();
            // Impossible to intercept during unit testing and mock.
        }

        private TypeInitialisedSingleton()
        {
            // Connect to Database...
            // Get data using a bunch of SQL statements...
        }

        public ISomeSingleton Default
        {
            get
            {
                return DefaultInstance;
            }
        }

        public void DoSomething()
        {
            // Omitted...
        }

        public string Stuff
        {
            get
            {
                // Omitted...
                return string.Empty;
            }
        }
    }
}

BAD BAD BAD.  There is no way to intercept the static constructor code during unit testing and inject mock behaviour.  As soon as the type is referenced by a piece of code the type initialiser will run.  I would avoid this at all costs.  Even if you don't plan to do unit testing, if you change you're mind later it may be hard to change. Generally speaking type initialisers (OO term for static constructors) should be avoided, and when used only used for simple static member initialisation.  If something goes wrong in the static constructor, even if handled, it can leave the application in a weird state (check this post).

A much improved version of a TypeInitialiserSingleton is the BishopSingleton (named after Judith Bishop and its from her book C# Design Patterns [O'Reilly])

namespace SingletonDemo
{
    public sealed class BishopSingleton : ISomeSingleton
    {
        // Local instance allows for tests to inject mock behaviour
        private static ISomeSingleton localInstance;

        private static class SingletonCreator
        {
            // Despite being internal, this is not visible outside BishopSingleton class.
            internal static readonly ISomeSingleton DefaultInstance;
            
            static SingletonCreator()
            {
                // if localInstance is not-null this will not be called.
                DefaultInstance = new BishopSingleton();
            }
        }

        private BishopSingleton()
        {
            // Initialisation...
        }

        public static ISomeSingleton Default
        {
            get
            {
                return localInstance ?? SingletonCreator.DefaultInstance;
            }
        }

        public void DoSomething()
        {
            // omitted...
        }

        public string Stuff
        {
            get
            {
                // Omitted...
                return string.Empty;
            }
        }
    }
}

This is quite elegant as it doesn't require understanding intricacies locking code and gives a means of tests injecting mock singletons (using private accessors). Although you will get Resharper and Code Analysis warnings that localInstance is never assigned, which feels a little dirty. Which is better? I personally prefer using SimpleSingleton when I know there is no thread contention and ContentionSingleton otherwise.  It is more obvious to the average developer what is going on inside the ContentionSingleton compared to the BishopSingleton in my humble opinion.

Finally, think once, twice, and thrice before using a Singleton, prefer using dependency injection, Inversion of Control, or a query or factory pattern, or even caching data in a shared way (MemCache or AppFabric).

Juval Löwy's (IDesign.net) Architecture Clinic

Recently I had the privilege of attending the IDesign Architecture Clinic in Sydney. It was a week long intensive mix of the architecture theory, the responsibilities of an architect, and practical coaching.  IDesign runs two similar courses in the software architecture stream: The Architect's Master-Class and the Architecture Clinic. The idea behind the Clinic is to keep the attendee numbers relatively small and have the opportunity to learn from a true master architect.  In Sydney my group was lucky to have Juval himself run the course.

Day one was full on.  Juval gave us a light speed overview of some of the critical material from the Architect's Master-class. The rest of the week was then practical hands-on experience using IDesign's "The Method". Days two, three and four followed the pattern of choose one of the attendee's systems and use The Method to draw up a proposed new architecture. Then Juval and the group will critique. Day five was all about project design, planning, and tracking.

The amount of knowledge and experience imparted during this course was simply incredible.  No other speaker I have ever listened to has packed this amount of information into five days.  There isn't more than a few handfuls of other people in the software industry that have the same level of industry insight and where it is heading in the next 10-20 years. During conversations there were more than a few statements that gave us a glimpse of where the industry is heading.  As a bonus Juval also kindly donated two of his evenings and run us through some other topics in depth, from the Architect's Master-class.

Anyone who has listened to Juval speak will know he is both highly intellectual and controversial. He often makes statements that send murmurs through an audience.  This course was no exception.  Some examples (not direct quotes):

• Every class as a WCF service.
• The Method doesn't fit well with Agile.
• The Architect is the responsible party not the project manager.
• Most project managers actively kill their projects.
• Self managing teams don't work.

For most questions the general answer was "there is no time, you need to attend the Architect's Master-class".  Other vague answers sounded as though he was hinting at some future technology now under development but is bound by NDA's. There were some answers, but with anything new and revolutionary every question reveals 3 more.

My personal opinion on this course was that it was worth while (it is definitely not a cheap course), but I would have liked to have completed the Architect's Master-class first.  I also believe that there was too much after-the-fact review based criticism and not enough coaching.  For days two, three, and four we were left to decompose the system and draw up our own plan for the whole day, whereas I would have preferred to have been walked through it from start to completion on day two before the practical on days three and four. Again just my personal opinion, and like I said still very worth while.

So, did I rush back to the office and evangelise all my co-workers and stakeholders? 

No. 

Why?

Because I cannot sell a new idea to my stakeholders without having prepared answers to the questions I know they will ask. Questions like: "We are a Scrum-Software-House, this doesn't fit with Scrum. You are not permitted to undertake projects in this way". Or "We hired you as an Architect not a project manager, you are operating outside your responsibilities". Or "Some of what you are prescribing infringes on senior developers responsibilities of technical design, architects should not be prescribing technical design".  And my favourite: "We don't have enough architects to operate this way and no-one wants to be an architect". The answers to these questions are not the kind you can Google and Juval simply recommends go to the Architect's Master-class.  So I'm filing this learning under "Interesting future insights that can't be used right now".

Can I justify going to the next Architect's Master-class?  

Probably not for a while.  Due to corporate politics its unlikely I can sell another course in the next 12 months just for me given the overall cost of fees, flights and accommodation (planned courses are only in the States). Also given that this one has not set my architecture team on fire with new levels of efficiency.

There's no easy solutions, just broad wholesale change.