Which is the better way to pass additional data for a changed workflow?

Question

Let entity A relates to entities B and C both as 1:1..*
At the same time B contains only part of C's key, but together with A's key it is possible to relate B to C.

Translation to human language:
Accounting Center (A) consists of Branches (C), and a stream of Payments (B) comes to the system from Accounting Center and contains BranchId which is unique only within a given Accounting Center. Wee need to check the correctness of provided BranchId and tie the Payment to the Branch.)

We process the collection of Bs:

class BsProcessor
{
    private BProcessor _processor;

    void ProcessBs(IEnumerable bs)
    {
          for (var b in bs)
          {
              _processor.Process(b);
          }
    }
}

One day requirements change, and now BProcessor needs to make a decision based on corresponding C.

From the view of performance it's better to get in advance all Cs for the As which Bs point to, and then pass this data to BProcessor via changing Process method signature.

class BsProcessor
{
    private BProcessor _processor;
    private CProvider _provider;

    void ProcessBs(IEnumerable bs)
    {
          var aKeys = bs.Select(b => b.aKey);
          var cs = _provider.GetCsForAs(aKeys).ToDictionary( c => c.aKey);

          for (var b in bs)
          {
              var cCandidates = cs[b.aKey];
              _processor.Process(b, cCandidates);
          }
    }
}

BProcessor then tries to find the matching C.

This is a straightforward fast to code and simple working solution...
To be honest I don't like it wholeheartedly. I think it violates SRP.
There is no any other reason but performance for the BsProcessor to be aware of Cs.

The logic of finding C candidates and matches belongs neither to BsProcessor nor to BProcessor, but a dedicated service CMatcher

class CMatcher
{
    private CProvider _provider;
    private IEnumerable<aKey> _aKeys;

    public CMatcher(CProvider provider, IEnumerable<aKey> aKeys)
    {
        ...
    }

    public C Match(aKey akey, cPartialKeyFromB partialKey)
    {
    } 
}

This service should be injected and used by BProcessor. As this service is contextual and requires collection of aKeys we need to switch to the factory:

class BsProcessor
{
    private BProcessorFactory _factory;

    void ProcessBs(IEnumerable bs)
    {
          var aKeys = b.Select(b => b.aKey);
          var processor = _factory.Create(aKeys);
          for (var b in bs)
          {
             processor.Process(b);
          }
    }
}

I think this solution is a bit more complex to code, but easier to test. The only problem I have is that BProcessor contains more than one method, and those do not require to match C to B, hence it is not good to use constructor injection, whereas method injection makes BsProcessor know about CMatcher what is somewhat similar to initial solution.

Here it starts looking like BProcessor begs for being divided into separate classes, and I ask myself whether SRP is worth so much refactoring and where second approach is indeed better than the first one?

Answer 1

The trick, I think, is to take the first approach, but pass in the CProvider during processing, rather than containing the CProvider at all times.

Also, depending on what else is involved in processing, you may not need to have BProcessor/BsProcessor classes at all; simply let each B be capable of processing itself, given a CProvider to find the corresponding C with.

Here's a quick and dirty example, using your accounting analogy above. Fair warning, I'm not validating much data, and I might be making bad assumptions about the requirements of the structures.

// A
class AccountingCenter
{
    private List<Branch> m_branches = new List<Branch>();

    public string Id { get; private set; }
    public ReadOnlyCollection<Branch> Branches { get { return m_branches.AsReadOnly(); } }

    public AccountingCenter(string id, IEnumerable<string> branchIds = null)
    {
        Id = id;
        if (branchIds != null)
        {
            foreach(var b in branchIds)
            {
                AddBranch(b);
            }
        }
    }

    public void AddBranch(string id)
    {
        m_branches.Add(new Branch(id, this));
    }
}

// C
class Branch
{
    private AccountingCenter m_parentCenter;

    public string BranchId { get { return m_parentCenter.Id + "-" + Id; } } // or whatever the combined implementation would be
    public string Id { get; private set; }

    public Branch(string id, AccountingCenter center)
    {
        Id = id;
        m_parentCenter = center;
    }
}

// CProvider
class AccountingCenterContainer
{
    private Dictionary<string, Branch> m_BranchIdToBranchMap = new Dictionary<string, Branch>();

    public AccountingCenterContainer(IEnumerable<AccountingCenter> centers)
    {
        foreach (var c in centers)
        {
            foreach (var b in c.Branches)
            {
                m_BranchIdToBranchMap.Add(b.BranchId, b);
            }
        }
    }

    public Branch GetBranchFromId(string branchId)
    {
        if (!m_BranchIdToBranchMap.ContainsKey(branchId))
        {
            throw new ArgumentException("ID " + branchId + " does not correspond to any known branch");
        }
        return m_BranchIdToBranchMap[branchId];
    }
}

// B
class Payment
{
    public string BranchId { get; private set; }

    public Payment(string branchId)
    {
        BranchId = branchId;
    }

    public void Process(AccountingCenterContainer container)
    {
        Branch b = container.GetBranchFromId(BranchId);
        // process...
    }
}

Answer 2

SRP is just a guidance, and it can be safely violated under concise decision. So if you don't think that the separation does not give direct benefit, it is fine to violate it.

But let's say that you want to separate the responsibility though. I see that your solution isn't optimal.

• First, it still need to know the involvement of aKey during getting the C in CMatcher. Why don't you pass IEnumerable<B> instead?
• Second, you change the BsProcessor's class signature, from originally provide them with BProcess now you are generating via a factory. I don't know what the benefit the factory bring, if it is justifiable then go on. But if not, why not use the initial signature? You can: 1. Add C property in each of B class, which is OO way and introduce coupling, or introduce another class which hold B and C, and pass the class into the BProcessor.