C# How to split the following large complex class.

Question

I've managed to program the following THWorkingMemory class, into a antipattern, the God Object. I planned it to be a fairly small class with around 20 methods, but now it's packed with queue handlers, timers, threads, callbacks, powershell queue callbacks, event handlers, lock handlers and about 50+ methods, i.e. the lot. The class has grown far too big to be maintainable, and I need to split into smaller classes. But how to do it?

The THWorkingMemory class fundamentally defines around 8 main code blocks, which would suggest 8 seperate classes, but all the methods which write to the TreeDictionary use the ReaderWriterLockerWrapper.

Here is the code.

interface IWorkingMemory
{
protected CMemory CBase;
protected CMemory CCM { get { .. } 
public abtract event .. 
public abstract void ExecuteAction(Guid ExecutionGuid, string jim ... ...);
 ..
..20+ methods, events 
}

internal sealed class CMemory
{
    public CMemory()
    {
     CBase=new TreeDictionary<Guid, ExecutionState>(new comparer);
    } ..
}


public sealed class ExecutionState 
{ // 20+ methods. that act against the treedictionary node  }

internal sealed class THWorkingMemory:IWorkingMemory
{   
    lockStrategy = new ReaderWriterLockerWrapper();

    public void ExecuteAction(Guid ExecutionGuid, string jim ... ...)
    {

     lockStrategy.AcquireWriteLock()
      CCM[ExecutionGuid].CreateExecutionState(jim);
     lockStrategy.ReleaseWriteLock()
    }

    2000 lines+ of methods, timers, threading, events, callbacks, 
        queues processing. powershell script callbacks from ExecutionState, etc.
}

private ReaderWriterLockerWrapper
{
    public void AcquireWriteLock(int timeout) {}\n
    public void ReleaseWriteLock() {}
}

I had a look at the questions regarding partial classes, but they don't get a good writeup. It would make sense here, as the ReaderWriterLockerWrapper is used by most methods in the THWorkingMemory class.

What's the best way of splitting the THWorkingMemory, so it preserves the veracity of the lock class, i.e. ensures that writes into the tree dictionary don't conflict, that is writes are locked. I also looked at nested classes, which would work as a solution, but not be able to use the locker in the same manner it is now.

Any ideas?

Answer 1

I also looked at nested classes, which would work as a solution, but not be able to use the locker in the same manner it is now.

Nested classes could use the locker if they were instantiated with a reference to the containing class, so that they could use the instance member data and/or invoke the instance methods of the containing class.

For example, given a skeleton like this ...

class THWorkingMemory
{
  class NestedClass
  {
    THWorkingMemory m_self;
    internal NestedClass(THWorkingMemory self)
    {
      m_self = self;
    }

    ... methods of NestedClass can invoke m_self.ExecuteAction
        and/or can access m_self.lockStrategy ...
  }

  NestedClass m_nestedClass;

  internal THWorkingMemory()
  {
    m_nestedClass = new NestedClass(this);
  }
}

... you can move methods/functionality out of the THWorkingMemory class and into the NestedClass.

---

Alternatively you can wrap only the to-be-shared data and methods into a class, and pass that a reference to that class (instead of a reference to the whole container) into your nested class[es].

class THWorkingMemory
{
  class SharedData
  {
    lockStrategy = new ReaderWriterLockerWrapper();

    public void ExecuteAction(Guid ExecutionGuid, string jim ... ...)
    {
      lockStrategy.AcquireWriteLock()
      CCM[ExecutionGuid].CreateExecutionState(jim);
      lockStrategy.ReleaseWriteLock()
    }
  }

  class NestedClass
  {
    SharedData m_sharedData;
    internal NestedClass(SharedData sharedData)
    {
      m_sharedData = sharedData;
    }

    ... methods of NestedClass can invoke m_sharedData.ExecuteAction
        and/or can access m_sharedData.lockStrategy ...
  }

  SharedData m_sharedData;
  NestedClass m_nestedClass;

  internal THWorkingMemory()
  {
    m_sharedData = new SharedData();
    m_nestedClass = new NestedClass(m_sharedData);
  }
}

---

EDIT

Well, The THWorkingMemory memory class implements IWorkingMemory inteface, so that interface be able to be passed to the nested classes?

I think you'd implement the interface in the main class by defining the methods in the main class, but implement those methods by delegating to corresponding methods in the nested classes, for example:

interface IWorkingMemory
{
  void SomeMethod();
  void AnotherMethod();
  ... + 20 other methods ...
}

class THWorkingMemory : IWorkingMemory
{
  class NestedClass
  {
    public void SomeMethod()
    {
      ... some complicated implementation here ...
    }

    ... + plus private methods which help to implement the public method ...
  }

  class AnotherNestedClass
  {
    public void AnotherMethod()
    {
      ... some complicated implementation here ...
    }

    ... + plus private methods which help to implement the public method ...
  }

  SharedData m_sharedData;
  NestedClass m_nestedClass;
  AnotherNestedClass m_anotherNestedClass;

  internal THWorkingMemory()
  {
    m_sharedData = new SharedData();
    m_nestedClass = new NestedClass(m_sharedData);
    m_anotherNestedClass = new AnotherNestedClass(m_sharedData);
  }

  #region implement IWorkingMemory methods

  public void SomeMethod()
  {
    //implement by delegating to the implementation in the nested class
    m_nestedClass.SomeMethod();
  }

  public void AnotherMethod()
  {
    //implement by delegating to the implementation in the nested class
    m_anotherNestedClass.AnotherMethod();
  }

  ... + 20 other methods ...

  #endregion
}

Note:

• Your main class is now simple/trivial: all complexity is encapsulated n the nested classes
• Each nested class might have private implementation details which it hides from other classes
• These nested classes might not need to be nested: you could make them "internal" classes instead
• Your main class has become a Facade.

Answer 2

I would look into the SOLID principles. Especially the single responsibility principle. I've found them to be very useful when analyzing a class to decide how to split it. I also use Fowler's refactoring catalog. Sometimes just by browsing it I'll hit upon a refactoring I hadn't thought to use.

Some general guidelines I use when examining a function:

• Does it rely on any private class fields? If it doesn't its an automatic candidate for moving to a separate class.
• Can the function be easily generalized? In other words, is its logic tied to the structure of the class data it manipulates or with small modifications could it be used elsewhere. If it can be generalized move it to a separate class.

Of course it helps tremendously if you have a good set of unit tests as a safety net for when you start moving things around.

You may also be interested in http://refactormycode.com/