How can I safely convert a static class to an instantiated class in C#

.net, architecture, c#

Solution

If you don't know anything about how this type flows in your application, it's a dangerous task. But if you really need to do that without, possibly, breaking everything, I would do like:

Knowing that what I need is a distinct devision between documents, and I know (it's proved by time) that this type works for a single document, let's add Document slice.

Let's assume `Document` has `Name` property, we can think about something like (example):

public static void AddNetObject(string documentName, NetObject newObject)
{
    ....
}

make all fields non static:

   //NO STATIC
    ...
    private Logger LogFile { get { return Logger.GetMethodLogger(2); } }   
    private Dictionary<string, NetObject> netObjectHashTable;
    private Dictionary<string, NetTitle> titlePropertyHashTable;
    private Dictionary<string, NetObject> referenceDataHashTable;
    private Dictionary<int, SortedDictionary<string, int>> picklistHashTable;

Move them into internal

private class NetDocument {
       public string DocumentName {get;set;} //DEFINE DOCUMENT IT RELATED TO !
       ...
       private Logger LogFile { get { return Logger.GetMethodLogger(2); } }   
       private Dictionary<string, NetObject> netObjectHashTable;
       ....

  }

So you create concrete isolation between single documents and related to them data.

After inside the main class yo can have:

public static class NetCollection
{
    ... 

    //Key: document name
    //Value: NetDocument data
    private Dictionary<string, NetDocument> documents = new ....
}

This is just a general idea (a sketch) , you for sure need to change it to fit your needs.

Problem

I have recently (a couple of months ago) changed jobs and have inherited a codebase which violates every single one of the SOLID principles as many times as it possibly can. It seems as though the people who wrote this code decided to study every single good coding practice in detail and violate them all as often and most radically as they possibly could. I am the sole developer of this product - there is no one left in the organisation who knows the code and the codebase is too large and complex to completely rewrite. I am looking at the highest value changes that I can make to make the codebase flexible and robust. It is not an option to abandon this product either. The root of all problems in the product stems from a group of classes which are the core business logic data structures. There is a lot of problems with these classes, but what I am really interested in is the following: ``` public static class NetCollection { private static Logger LogFile { get { return Logger.GetMethodLogger(2); } } // Declare local variables. private static Dictionary<string, NetObject> netObjectHashTable; private static Dictionary<string, NetTitle> titlePropertyHashTable; private static Dictionary<string, NetObject> referenceDataHashTable; private static Dictionary<int, SortedDictionary<string, int>> picklistHashTable; public static IEnumerable<NetObject> NetObjects { get { return netObjectHashTable.Values; } } static NetCollection() { netObjectHashTable = new Dictionary<string, NetObject>(); titlePropertyHashTable = new Dictionary<string, NetTitle>(); referenceDataHashTable = new Dictionary<string, NetObject>(); picklistHashTable = new Dictionary<int, SortedDictionary<string, int>>(); } public static void AddNetObject(NetObject newObject) { if (newObject == null) return; if (newObject.TitleType == "Reference Data") { // Check if hash table contains key if (!referenceDataHashTable.ContainsKey(newObject.ID.ToString())) { referenceDataHashTable.Add(newObject.ID.ToString(), newObject); } } else { // Check if hash table contains key if (!netObjectHashTable.ContainsKey(newObject.ID.ToString())) { netObjectHashTable.Add(newObject.ID.ToString(), newObject); } } } } ``` I have snipped quite a number of other methods from this class for the sake of brevity. As you can see, there are a huge number of issues around this class (storing state in a static class is a huge code smell - writing your entire application around said class is just crazy). My current intention is to refactor this class into a proper singleton class (and eventually into a regular class so that I can enable the user to open multiple documents simultaneously). Should I do this? What are the biggest risks with making this change? Are there any approaches that I can take to mitigate the risk of making this change?

Original source