How to split code into components... big classes? small classes?

Question

This is very basic stuff, but here goes. I find that I am never able to agree with myself whether the way I split large classes into smaller ones make things more maintainable or less maintainable. I am familiar with design patterns, though not in detail, and also with the concepts of object oriented design. All the fancy rules and guidelines aside, I am looking to pick your brains with regards to what I am missing with a very simple, sample scenario. Essentially along the lines of: "...this design will make it more difficult to", etc... All the things I don't anticipate because of lack of experience essentially.

Say you need to write a basic "file reader / file writer" style class to process a certain type of file. Let's call the file a YadaKungFoo file. The contents of YadaKungFoo files is essentially similar to INI Files, but with subtle differences.

There are sections and values:

[Sections]
Kung,Foo
Panda, Kongo

[AnotherSection]
Yada,Yada,Yada
Subtle,Difference,Here,Yes

[Dependencies]
PreProcess,SomeStuffToPreDo
PreProcess,MoreStuff
PostProcess,AfterEight
PostProcess,TheEndIsNear
PostProcess,TheEnd

OK, so this can yield 3 basic classes:

public class YadaKungFooFile
public class YadaKungFooFileSection
public class YadaKungFooFileSectionValue

The two latter classes are essentially only data structures with a ToString() override to spit out a string list of values stored using a couple of Generic lists. This is sufficient to implement the YadaKungFooFile save functionality.

So over time the YadaYadaFile starts growing. Several overloads to save in different formats, including XML etc, and the file starts pushing towards 800 lines or so. Now the real question: We want to add a feature to validate the contents of a YadaKungFoo file. The first thing that comes to mind is obviously to add:

var yada = new YadaKungFooFile("C:\Path");
var res = yada .Validate()

And we're done (we can even call that method from the constructor). Trouble is the validation is quite complicated, and makes the class very large, so we decide to create a new class like this:

var yada = new YadaKungFooFile("C:\Path"); 
var validator = new YadaKungFooFileValidator(yada); 
var result = validator.Validate();

Now this sample is obviously terribly simple, trivial and insignificant. Either of the two ways above probably won't make too much difference, but what I don't like is:

1. The YadaKungFooFileValidator class and the YadaKungFooFile class seem to be very strongly coupled by this design. It seems a change in one class, would likely trigger changes in the other.
2. I am aware that phrases such as "Validator", "Controller", "Manager" etc... indicates a class that is concerned with the state of other objects as opposed to its "own business", and hence violates separation of concern principles and message sending.

All in all I guess I feel that I don't have the experience to understand all the aspects of why a design is bad, in what situations it doesn't really matter and what concern carries more weight: smaller classes or more cohesive classes. They seem to be contradictive requirements, but perhaps I am wrong. Maybe the validator class should be a composite object?

Essentially I am asking for comments on likely benefits / problems that could result from the above design. What could be done differently? (base FileValidator class, FileValidator Interface, etc.. u name it). Think of the YadaKungFooFile functionality as ever growing over time.

Answer 1

I'll address this

The YadaKungFooFileValidator class and the YadaKungFooFile class seem to be very strongly coupled by this design. It seems a change in one class, would likely trigger changes in the other.

Yes, so in light of that, figure out how to make that as seamless as possible. In the best case, design the YadaKungFooFile class in such a way that the validator can pick up on that on it's own.

First, the syntax itself should be pretty easy. I wouldn't expect the syntax validation to change so you're probably safe to just hard code that.

As far as the acceptable properties, you could expose Enumerators from the File class that the Validator will check against. If a parsed value isn't in any of the enumerators for a given section, throw an exception.

So on and so forth...

Answer 2

I don't think the size of a class is a concern. The concern is more cohesion and coupling. You want to design objects that are loosely coupled and cohesive. That is, they should be concerned with one well defined thing. So if the thing happens to be very complicated then the class will grow.

You can use various design patterns to help manage the complexity. One thing you can do for instance is to create a Validator interface and have the YadaKunfuFile class depend on the interface rather than the Validator. This way you can change the Validator class without changes to the YadaKungfuFile class as long as the interface does not change.

Answer 3

My personal way of handling things in this case would be to have a class YadaKungFooFile which would be made up of a list of YadaKungFooFileSections, which would be a list of YadaKungFooFileValues.

In the case of the validator, you would have each class call the validate method of the class below it, with the lowest class in the hierarchy implement the actual meat of the validation method.

Answer 4

If the validation does not depend on the whole file but instead only works on individual section values, you can decompose the validation of the YadaKungFooFile class by specifying a set of validators operating on section values.

If the validation does depend on the whole file the validation is naturally coupled to the file class and you should consider putting it in one class.

Answer 5

Bob Martin wrote a series of articles on class design, which he refers to as the SOLID principles:

http://butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod

The principles are:

1. The Single Responsibility Principle: A class should have one, and only one, reason to change.
2. The Open-Closed Principle: You should be able to extend a classes behavior, without modifying it.
3. The Liskov Substitution Principle: Derived classes must be substitutable for their base classes.
4. The Interface Segregation Principle: Make fine grained interfaces that are client specific.
5. The Dependency Inversion Principle: Depend on abstractions, not on concretions.

So in light of those, let's look at some of the statements:

So over time the YadaYadaFile starts growing. Several overloads to save in different formats, including XML etc, and the file starts pushing towards 800 lines or so

This is a first Big Red Flag: YadaYadaFile starts out with two responsibilities: 1) maintaining a section/key/value data structure, and 2) knowing how to read and write an INI-like file. So there's the first problem. Adding more file formats compounds the issue: YadaYadaFile changes when 1) the data structure changes, or 2) a file format changes, or 3) a new file format is added.

Similarly, having a single validator for all three of those responsibilities puts too much responsibility on that single class.

A large class is a "code smell": it's not bad in and of itself, but it very often results from something that really is bad--in this case, a class that tries to be too many things.

Answer 6

Class size is not as much of an issue as method size. Classes are a means of organization and while there are good and bad ways to organize your code, it is not tied directly to class size.

Method size is different since a method is doing the actual work. Methods should have very specific jobs which will inevitably limit their size.

I find that the best way to determine method size is through the writing of unit tests. If you are writing unit tests with sufficient code coverage then it becomes apparent when a method is too big.

Unit test methods need to be simple or else you will end up needing tests to test your unit tests to know they are working properly. If your unit test methods are becoming complicated, it is likely due to the fact that a method is trying to do too much and should be broken into smaller methods with clear responsibilities.

Answer 7

YadaKungFooFile should not know how to read itself from the disk. It should only know how to traverse itself, disclose its children etc. It should also provide ways to add / remove children etc.

There should be IYadaKungFooReader interface that YadaKungFooFile would take in his Load method that it will use to load itself from disk. Plus several implementations like AbstractKungFooReader, PlainTextYadaKungFooReader, XMLYadaKungFooWriter that know how to read and corresponding formats.

Same for writing.

Finally, there should be YadaKungFooValidatingReader that will take an IYadaKungFooReader reader in, use it for reading and validate the input as it reads. Then you would pass the validating reader to the YadaKungFooFile.Load whenever you want it to validate as it reads from disk.

Alternatively, you could make the reader active, instead of passive class. Then, instead of passing it to YadaKungFooFile, you would use it as a factory that would create your YadaKungFooFile using latter normal access methods. In this case your reader should also implement YadaKungFooFile interface so that you can chain normal reader -> validating reader -> YadaKungFooFile. Makes sense?