Gfyuki

A friend and I (both new in programming) were arguing if it was wise to include a Dictionary inside a Class that contains all instances of that class.

I say that is better that the dictionary is in the main instead in the class itself.

I have no strong reasons to say why , but i've never seen classes keeping records of the objects that has created.

Could you give me pros and cons of each approach?

Thanks in advance.

A:

    class Table

    {

      public static Dictionary<int,Table> Tables = new Dictionary<int, Table>();

     ...

     public table (int ID)  // constructor

     {

      ...

      Tables.Add(ID,this);

     }   

    }

B:

     class Program

     {

      public Dictionary<int,Table> Tables = new Dictionary<int, Table>();

      static void Main(string args)

      {

       ...

       Table A = new Table (10);

       Tables.Add(10,A);

       }

      }

It mostly depends on your needs and your architectural/design preferences.

Having the dictionary inside your class makes all class-related logic nicely encapsulated. That way, you can hide the (static) dictionary from the class users and have it managed by your class internally.

Having the dictionary outside your class makes the mechanism flexible in other ways. You can manage multiple different dictionaries for your class instances (for multiple purposes), for instance. Or you can just leave such a dictionary away if you don't need it in a specific solution environment.

IMHO, there are no strict guidelines that tell you that you should or shouldn't do something specific. Just be creative. As long as the end result is clear, maintainable, extendable etc. the sky is the limit.

Well, A version means that you can't have two Table with a same ID.

using System.Collections.Concurrent;



...



public class Table {

  //DONE: Do not expose fields but readonly properties 

  //DONE: Keep static (i.e. global) members (fields, properties, methods) being thread safe

  private static ConcurrentDictionary<int, Table> s_Tables = 

    new ConcurrentDictionary<int, Table>();



  public Table(int ID) { 

    s_Tables.Add(ID, this);

  }



  //DONE: All we expose is thead safe read-only version of the dictionary

  public static IReadOnlyDictionary<int, Table> Tables = s_Tables;

}

When B version means that you can well have several Program each of them have their own Tables and that's why ID is not globaly unique:

public class Program {

  //DONE: Do not expose fields but readonly properties 

  private Dictionary<int,Table> m_Tables = new Dictionary<int, Table>();



  public Program() {

    Table A = new Table (10);

    m_Tables.Add(10,A); 



    ...

  }



  //DONE: All we expose is read-only version of the dictionary

  public IReadOnlyDictionary<int, Table> Tables = m_Tables; 

}



...



//DONE: please, do not cram all the logic into `Main`, 

//      have a special classes business logic (Program) and entry point

public static class EntryPoint {

  static void Main(string args) {

    Program program = new Program();



    ...

  }

}

Since in your original code you have static void Main in Program class it's efficiently singleton, so it seems, version A is preferable: all about Table are within Table class; you can't occasionaly create a second instance of Table with same ID

Approach A. If all instances of a class should be tracked then it should be an automatic feature of the class which can be implemented using a static dictionary. The dictionary should be private so that other classes cannot modify it. This gives you a guarantee that the dictionary will be correct.

However, this is not a good technique for beginner programmers because it prevents the objects from being cleaned up by garbage collection. This means you have to manually decide when the objects should be released, at which time you remove them from the dictionary. You could implement the iDisposable interface for this. Usually it's best to use a using block with it, but sometimes you will need to find another way to manage the lifetime of the objects.

Approach B is fine for a small program, but it does not have any advantages either. In a larger program it will be hard to manage and prone to mistakes. You may forget to add an object, or you may have some code incorrectly add or remove an object, or some code may assume an object is there when it was removed, etc...

It's nothing wrong with any concepts. You have to decide whether caching is part of the design (A) or is an option of a consumer layer (B).

If it is a built-in feature I would slightly modify approach A:

public class Table

{

    // cache is private so it cannot be manipulated from outside

    private static Dictionary<int,Table> tables = new Dictionary<int, Table>();



    // constructor is private so Table can be created by the GetTable factory method

    private Table(int id)

    {

        //...

    }



    // the factory method that transparently returns the table either from cache or by creating one

    public static Table GetTable(int id)

    {

         if (tables.TryGetValue(id, out Table result))

             return result;

         result = new Table(id);

         tables[id] = result;

         return result;

    }

}

A:
pros: none
cons:

• it is not the responsibility of class to track its instances (see SRP)




• non-thread-safe (see ConcurrentDictionary)

B:
pros:

• class is not burdened excess responsibilities that out its scope

cons:

• 'tracking'-code should be encapsulated to separate class that be responsible for creating instances, tracking them, managing time life strategy, etc (factory may fit for that)


• non-thread-safe

As mentioned above If you need to cache some items then use the MemoryCache.