Managing and organizing the massively increased number of classes after switching to SOLID?

Question

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.

Answer 1

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.

Answer 2

Tasks that used to take 5-10 files can now take 70-100!

This is the opposite of the single-responsibility principle (SRP). To get to that point, you must have divided up your functionality in a very fine-grained way, but that's not what the SRP is about -- doing that ignores the key idea of cohesiveness.

According to the SRP, software should be divided into modules along lines defined by their possible reasons to change, so that a single design change can be applied in just one module without requiring modifications elsewhere. A single "module" in this sense may correspond to more than one class, but if one change requires you to touch tens of files then either it's really multiple changes or you're doing SRP wrong.

Bob Martin, who originally formulated the SRP, wrote a blog post a few years ago to try to clarify the situation. It discusses at some length what a "reason to change" is for the purposes of the SRP. It's worth a read in its entirety, but among the things within deserving special attention is this alternative wording of the SRP:

Gather together the things that change for the same reasons. Separate those things that change for different reasons.

(emphasis mine). The SRP is not about splitting things up into the tiniest possible pieces. That's not good design, and your team is right to resist. It makes your code base harder to update and to maintain. It sounds like you may be trying to sell your team on it based on unit testing considerations, but that would be putting the cart before the horse.

Similarly, the interface segregation principle should not be taken as an absolute. It is no more a reason to divide your code so finely than is the SRP, and it generally aligns pretty well with the SRP. That an interface contains some methods that some clients do not use is not a reason to break it up. You are again looking for cohesion.

Additionally, I urge you not to take the open-closed principle or the Liskov substitution principle as a reason to favor deep inheritance hierarchies. There is no tighter coupling than a subclass with its superclasses, and tight coupling is a design problem. Instead, favor composition over inheritance wherever it makes sense to do so. This will reduce your coupling, and thus the number of files a particular change may need to touch, and it aligns nicely with dependency inversion.

Answer 3

Tasks that used to take 5-10 files can now take 70-100!

This is a lie. The tasks never took only 5-10 files.

You are not solving any tasks with less than 10 files. Why? Because you're using C#. C# is a high level language. You are using more than 10 files just to create hello world.

Oh sure you don't notice them because you didn't write them. So you don't look in them. You trust them.

The problem isn't the number of files. It's that you now have so much going on that you don't trust.

So figure out how to make those tests work to the point that once they pass you trust these files the way you trust the files in .NET. Doing that is the point of unit testing. No one cares about the number of files. They care about the number of things they can't trust.

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.

Change is hard on very large scale applications no mater what you do. The best wisdom to apply here doesn't come from Uncle Bob. It comes from Michael Feathers in his book Working Effectively with Legacy Code.

Do not start a rewrite fest. The old code represents hard won knowledge. Tossing it out because it has problems and isn't expressed in new and improved paradigm X is just asking for a new set of problems and no hard won knowledge.

Instead find ways to make testable your old untestable code (legacy code in Feathers speak). In this metaphor code is like a shirt. Large parts are joined at natural seams that can be undone to separate the code the way you'd remove the seams. Do this to allow you to attach test "sleeves" that let you isolate the rest of the code. Now when you create the test sleeves you have confidence in the sleeves because you did this with a working shirt. (ow, this metaphor is starting to hurt).

This idea flows from the assumption that, like in most shops, the only up to date requirements are in the working code. This lets you lock that down in tests that allow you to make changes to the proven working code without it losing every bit of it's proven working status. Now with this first wave of tests in place you can start making changes that make the "legacy" (untestable) code testable. You can be bold because the seams tests are backing you up by saying this is what it always did and the new tests show that your code actually does what you think it does.

What does any of this have to do with:

Managing and organizing the massively increased number of classes after switching to SOLID?

Abstraction.

You can make me hate any code base with bad abstractions. A bad abstraction is something that makes me look inside. Don't surprise me when I look inside. Be pretty much what I expected.

Give me a good name, readable tests (examples) that show how to use the interface, and organize it so I can find things and I wont care if we used 10, 100, or 1000 files.

You help me find things with good descriptive names. Put things with good names in things with good names.

If you do all this right, you will abstract the files to where finishing a task has you only depending on 3 to 5 other files. The 70-100 files are still there. But they're hiding behind the 3 to 5. That only works if you trust the 3 to 5 to do that right.

So what you really need is the vocabulary to come up with good names for all these things and tests that people trust so they'll stop wading through everything. Without that you'd be making me crazy too.

@Delioth makes a good point about growing pains. When you're used to the dishes being in the cupboard above the dish washer it takes some getting used to them being above the breakfast bar. Makes some things harder. Makes some things easier. But it causes all kinds of nightmares if people don't agree where the dishes go. In a large code base the problem is you can only move some of the dishes at a time. So now you have dishes in two places. It's confusing. Makes it hard to trust that the dishes are where they're supposed to be. If you want to get past this though the only thing to do is keep moving the dishes.

The problem with that is you'd really like to know if having the dishes over the breakfast bar is worth it before going through all this nonsense. Well for that all I can recommend is go camping.

When trying out a new paradigm for the first time the last place you should be applying it is in a large code base. This goes for every member of the team. No one should be taking it on faith that SOLID works, that OOP works, or that functional programming works. Every team member should get a chance to play with the new idea, whatever it is, in a toy project. It lets them see at least how it works. It lets them see what it doesn't do well. It lets them learn to do it right before they make a big mess.

Giving people a safe place to play will help them adopt new ideas and give them confidence that the dishes really could work in their new home.

Answer 4

It sounds as though your code isn't very well decoupled and/or your task sizes are way too big.

Code changes should be 5-10 files unless you're doing a codemod or large scale refactoring. If a single change touches a lot of files, it probably means that your changes cascade. Some improved abstractions (more single responsibility, interface segregation, dependency inversion) should help. It's also possible you maybe went too single responsibility and could use a bit more pragmatism - shorter and thinner type hierarchies. That should make the code easier to understand too since you don't have to understand dozens of files to know what the code is doing.

It also might be a sign that your work is too big. Instead of "hey, add this feature" (which requires UI changes and api changes and data access changes and security changes and test changes and...) break it down into more serviceable chunks. That becomes easier to review and easier to understand because it requires you to set up decent contracts between the bits.

And of course, unit tests help all of this. They force you to make decent interfaces. They force you to make your code flexible enough to inject the bits needed to test (if it's hard to test, it'll be hard to reuse). And they push people away from over-engineering stuff because the more you engineer the more you need to test.

Answer 5

I would like to expound on some of the things already mentioned here, but more from a perspective of where object boundaries are drawn. If you're following something akin to Domain-Driven Design, then your objects are probably going to represent aspects of your business. Customer and Order, for example, would be objects. Now, if I were to make a guess based on the class names that you had as your starting point, your AccountLogic class had code that would run for any account. In OO, however, each class is meant to have context and identity. You should not get an Account object and then pass it into an AccountLogic class and have that class make changes to the Account object. That is what is called an anemic model, and doesn't represent OO very well. Instead, your Account class should have behavior, such as Account.Close() or Account.UpdateEmail(), and those behaviors would affect only that instance of the account.

Now, HOW these behaviors are handled can (and in a lot of cases should) be off-loaded to dependencies represented by abstractions (ie, interfaces). Account.UpdateEmail, for example, might want to update a database, or a file, or send a message to a service bus, etc. And that might change in the future. So your Account class may have a dependency on, for example, an IEmailUpdate, which could be one of many interfaces implemented by an AccountRepository object. You wouldn't want to pass a whole IAccountRepository interface to the Account object because it would probably do too much, such as search and find other (any) accounts, which you may not want the Account object to have access to, but even though AccountRepository might implement both IAccountRepository and IEmailUpdate interfaces, the Account object would only have access to the small portions that it needs. This helps you maintain the Interface Segregation Principle.

Realistically, as other people have mentioned, if you're dealing with an explosion of classes, chances are that you're using SOLID principle (and, by extension, OO) the wrong way. SOLID should be helping you simplify your code, not complicate it. But it takes time to really understand what things like the SRP mean. The more important thing, though, is that how SOLID works is going to be very dependent on your domain and bounded contexts (another DDD term). There's no silver bullet or one-size-fits-all.

One more thing that I like to emphasize to people I work with: again, an OOP object should have behavior, and is, in fact, defined by its behavior, not its data. If your object has nothing but properties and fields, it still has behavior, though probably not the behavior you intended. A publicly-writable/settable property with no other set logic implies that the behavior for its containing class is that anyone anywhere for whatever reason and at any time is allowed to modify the value of that property without any necessary business logic or validation in between. That's not usually the behavior people intend, but if you have an anemic model, that's generally the behavior your classes are announcing to anyone using them.

Answer 6

So that's a total of 15 classes (excluding POCOs and scaffolding) to perform a fairly straightforward save.

That's crazy.... but these classes sound like something I'd write myself. So let's have a look at them. Let's ignore the interfaces and tests for now.

• BasePathProvider - IMHO any non-trivial project working with files needs it. So I'd assume, there's already such a thing and you can use it as is.
• UniqueFilenameProvider - Sure, you have it already, haven't you?
• NewGuidProvider - The same case, unless you're just staring to use GUID.
• FileExtensionCombiner - The same case.
• PatientFileWriter - I guess, this is the main class for the current task.

To me, it looks good: You need to write one new class which needs four helper classes. All four helper classes sound pretty reusable, so I'd bet they're already somewhere in you code base. Otherwise, it's either bad luck (are you really the person in your team to write files and use GUIDs???) or some other problem.

---

Concerning the test classes, sure, when you create a new class, or update it, it should be tested. So writing five classes means writing five test classes, too. But this doesn't make the design any more complicated:

• You won't ever use the test classes elsewhere as they will get executed automatically and that's all.
• You want ever look at them again, unless you update the classes under test or unless you use them as documentation (ideally, the tests show clearly how a class is supposed to be used).

---

Concerning the interfaces, they're only needed when either your DI framework or your testing framework can't deal with classes. You may see them as a toll for imperfect tools. Or you may see them as a useful abstraction allowing you to forget that there are more complicated things - reading the source of an interface takes much less time than reading the source of its implementation.

Answer 7

Depending on abstractions, creating single-responsibility classes, and writing unit tests are not exact sciences. It's perfectly normal to swing too far in one direction when learning, go to an extreme, and then find a norm that makes sense. It just sounds like your pendulum has swung too far, and might even be stuck.

Here's where I suspect this is going off the rails:

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.

One of the benefits that comes from most SOLID principles (certainly not the only benefit) is that it makes writing unit tests for our code easier. If a class depends on an abstractions we can mock the abstractions. Abstractions which are segregated are easier to mock. If a class does one thing it's likely to have lower complexity, which means it's easier to know and test all of its possible paths.

If your team isn't writing unit tests, two related things are happening:

First, they are doing a lot of extra work to create all of these interfaces and classes without realizing the full benefits. It takes a little time and practice to see how writing unit tests makes our lives easier. There are reasons why people who learn to write unit tests stick to it, but you have to persist long enough to discover them for yourself. If your team isn't attempting that then they're going to feel like the rest of the extra work they're doing is useless.

For example, what happens when they need to refactor? If they've got a hundred little classes but no tests to tell them whether or not their changes will work, those extra classes and interfaces are going to seem like a burden, not an improvement.

Second, writing unit tests can help you to understand how much abstraction your code really needs. Like I said, it's not a science. We start off badly, veering all over the place, and get better. Unit tests have a peculiar way of complementing SOLID. How do you know when you need to add an abstraction or break something apart? In other words, how do you know when you're "SOLID enough?" Often the answer is when you can't test something.

Maybe your code would be testable without creating as many tiny abstractions and classes. But if you're not writing the tests, how can you tell? How far do we go? We can become obsessed with breaking things up smaller and smaller. It's a rabbit hole. The ability to write tests for our code helps us see when we've accomplished our purpose so that we can stop obsessing, move on, and have fun writing more code.

Unit tests aren't a silver bullet that solves everything, but they are a really awesome bullet that makes developers' lives better. We're not perfect, and neither are our tests. But tests give us confidence. We expect our code to be right and we're surprised when it's wrong, not the other way around. We're not perfect and neither are our tests. But when our code is tested we have confidence. We're less likely to bite our nails when our code is deployed and wonder what's going to break this time and whether it's going to be our fault.

On top of that, once we get the hang of it, writing unit tests makes developing code faster, not slower. We spend less time revisiting old code or debugging to find problems that are like needles in a haystack.

Bugs decrease, we get more done, and we replace anxiety with confidence. It's not a fad or snake oil. It's real. Many developers will attest to this. If your team hasn't experienced this, they need to push through that learning curve and get over the hump. Give it a chance, realizing that they won't get results instantly. But when it happens they'll be glad they did and they'll never look back. (Or they'll become isolated pariahs and write angry blog posts about how unit tests and most other accumulated programming knowledge is a waste of time.)

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.

Peer review is a lot easier when all of the unit tests pass and a big part of that review is just making sure that the tests are meaningful.