C# – Managing and organizing the massively increased number of classes after switching to SOLID

ccoding-styledependency-injectionsingle-responsibilitysolid

Over the last few years, we have been slowly making the switch over to progressively better written code, a few baby steps at a time. We are finally starting to make the switch over to something that at least resembles SOLID, but we're not quite there yet. Since making the switch, one of the biggest complaints from developers is that they can't stand peer reviewing and traversing dozens and dozens of files where previously every task only required the developer touching 5-10 files.

Prior to starting to make the switch, our architecture was organized pretty much like the following (granted, with one or two orders of magnitude more files):

Solution
- Business
-- AccountLogic
-- DocumentLogic
-- UsersLogic
- Entities (Database entities)
- Models (Domain Models)
- Repositories
-- AccountRepo
-- DocumentRepo
-- UserRepo
- ViewModels
-- AccountViewModel
-- DocumentViewModel
-- UserViewModel
- UI

File wise, everything was incredibly linear and compact. There was obviously a lot of code-duplication, tight-coupling, and headaches, however, everyone could traverse it and figure it out. Complete novices, people who had never so much as opened Visual Studio, could figure it out in just a few weeks. The lack of overall file complexity makes it relatively straightforward for novice developers and new hires to start contributing without too much ramp up time as well. But this is pretty much where any benefits of the code style go out the window.

I wholeheartedly endorse every attempt we make to better our codebase, but it is very common to get some push-back from the rest of the team on massive paradigm shifts like this. A couple of the biggest sticking points currently are:

  • Unit Tests
  • Class Count
  • Peer Review Complexity

Unit tests have been an incredibly hard sell to the team as they all believe they're a waste of time and that they're able to handle-test their code much quicker as a whole than each piece individually. Using unit tests as an endorsement for SOLID has mostly been futile and has mostly become a joke at this point.

Class count is probably the single biggest hurdle to overcome. Tasks that used to take 5-10 files can now take 70-100! While each of these files serve a distinct purpose, the sheer volume of files can be overwhelming. The response from the team has mostly been groans and head scratching. Previously a task may have required one or two repositories, a model or two, a logic layer, and a controller method.

Now, to build a simple file saving application you have a class to check if the file already exists, a class to write the metadata, a class to abstract away DateTime.Now so you can inject times for unit testing, interfaces for every file containing logic, files to contain unit tests for each class out there, and one or more files to add everything to your DI container.

For small to medium size applications, SOLID is a super easy sell. Everyone sees the benefit and ease of maintainability. However, they're just not seeing a good value proposition for SOLID on very large scale applications. So I'm trying to find ways to improve organization and management to get us past the growing pains.

I figured I'd give a bit stronger of an example of the file volume based on a recently completed task. I was given a task to implement some functionality in one of our newer microservices to receive a file sync request. When the request is received, the service performs a series of lookups and checks, and finally saves the document to a network drive, as well as 2 separate database tables.

To save the document to the network drive, I needed a few specific classes:

- IBasePathProvider 
-- string GetBasePath() // returns the network path to store files
-- string GetPatientFolderName() // gets the name of the folder where patient files are stored
- BasePathProvider // provides an implementation of IBasePathProvider
- BasePathProviderTests // ensures we're getting what we expect

- IUniqueFilenameProvider
-- string GetFilename(string path, string fileType);
- UniqueFilenameProvider // performs some filesystem lookups to get a unique filename
- UniqueFilenameProviderTests

- INewGuidProvider // allows me to inject guids to simulate collisions during unit tests
-- Guid NewGuid()
- NewGuidProvider 
- NewGuidProviderTests

- IFileExtensionCombiner // requests may come in a variety of ways, need to ensure extensions are properly appended.
- FileExtensionCombiner
- FileExtensionCombinerTests

- IPatientFileWriter
-- Task SaveFileAsync(string path, byte[] file, string fileType)
-- Task SaveFileAsync(FilePushRequest request) 
- PatientFileWriter
- PatientFileWriterTests

So that's a total of 15 classes (excluding POCOs and scaffolding) to perform a fairly straightforward save. This number ballooned significantly when I needed to create POCOs to represent entities in a few systems, built a few repos to communicate with third party systems that are incompatible with our other ORMs, and built logic methods to handle the intricacies of certain operations.

Best Answer

Now, to build a simple file saving application you have a class to check if the file already exists, a class to write the metadata, a class to abstract away DateTime.Now so you can inject times for unit testing, interfaces for every file containing logic, files to contain unit tests for each class out there, and one or more files to add everything to your DI container.

I think you have misunderstood the idea of a single responsibility. A class's single responsibility might be "save a file". To do that, it then may break that responsibility down into a method that checks whether a file exists, a method that writes metadata etc. Each those methods then has a single responsibility, which is part of the class's overall responsibility.

A class to abstract away DateTime.Now sounds good. But you only need one of those and it could be wrapped up with other environment features into a single class with the responsibility for abstracting environmental features. Again a single responsibility with multiple sub-responsibilities.

You do not need "interfaces for every file containing logic", you need interfaces for classes that have side-effects, e.g. those classes that read/write to files or databases; and even then, they are only needed for the public parts of that functionality. So for example in AccountRepo, you might not need any interfaces, you might only need an interface for the actual database access which is injected into that repo.

Unit tests have been an incredibly hard sell to the team as they all believe they're a waste of time and that they're able to handle-test their code much quicker as a whole than each piece individually. Using unit tests as an endorsement for SOLID has mostly been futile and has mostly become a joke at this point.

This suggests that you have misunderstood unit tests too. The "unit" of a unit test is not a unit of code. What even is a unit of code? A class? A method? A variable? A single machine instruction? No, the "unit" refers to a unit of isolation, i.e. code that can execute in isolation from other parts of the code. A simple test of whether an automated test is a unit test or not is whether you can run it in parallel with all your other unit tests without affecting its result. There's a couple more rules of thumb around unit tests, but that is your key measure.

So if parts of your code can indeed be tested as a whole without affecting other parts, then do that.

Always be pragmatic and remember everything is a compromise. The more you adhere to DRY, the more tightly coupled you code must become. The more you introduce abstractions, the easier the code is to test, but the harder it is to understand. Avoid ideology and find a good balance between the ideal and keeping it simple. There lies the sweet spot of maximum efficiency both for development and maintenance.

Related Topic