Classes to avoid (code complete)

Question

I am somewhat confused about a paragraph in the code complete book.

In the section "Classes to avoid" it reads:

"Avoid classes named after verbs A class that has only behavior but no data is generally not really a class. Consider turning a class like DatabaseInitialization() or StringBuilder() into a routine on some other class"

My code mainly consists of verb classes without data. There are invoicereaders, pricecalculators, messagebuilders etc. I do this to concentrate the classes to one task each. Then I add dependencies to other classes for other functionality.

If I understand the paragraph correctly I should use code like

class Webservice : IInvoiceReader, IArticleReader {
    public IList<Invoice> GetInvoices();
    public IList<Article> GetArticles();
}

rather than

class InvoiceReader : IInvoiceReader {
    public InvoiceReader(IDataProvider dataProvider);
    public IList<Invoice> GetInvoices();
}

class ArticleReader : IArticleReader {
    public ArticleReader(IDataProvider dataProvider);
    public IList<Article> GetArticles();
}

Edit Thanks for all the replies.

My conclusion is that my current code is more SRP than OO but that it also suffers from the "anemic domain model".

I'm sure theses insights will help me in future.

Answer 1

Focus less on the name. The rule about the name is just a rule-of-thumb indicator for a poor practice. The important point is this:

A class that has only behavior but no data is generally not really a class

In your case, it looks as though your classes have both data and behavior, and that they may as well be called "Invoice" and "Article".

Answer 2

It depends. A lot of classes exist named after Read and Write verbs because the classes also create, maintain, and represent a connection to the data source they're reading to or writing from. If your classes are doing that, it is probably best to keep them separate.

If the Reader objects just contain parsing logic, then turning the classes into utility methods is the way to go. I'd go with a more descriptive name than Webservice though.

Answer 3

Essentially what the book is saying is that OO design is about extracting the objects (the nouns) and identifying the operations (the verbs) that occur on and between those objects.

The nouns become the objects, the verbs become the methods that operate on these objects.

The idea is that the

closer the program models the real world problem, the better the program will be.

In practical terms the useful thing with an object is that it can represent a particular state. Then you can have several different instances of this class each holding a different state to represent some aspect of the problem.

In the case of the InvoiceReader class

• you are only going to be creating one instance
• the only state it represents is that of containing a dataProvider
• it only contains one method

there is no advantage to placing it in an object.

Answer 4

I think the book suggests a design like the following:

class Article : IIArticleReader
{
    // Article data...

    public IList<Article> GetArticles(); 
}

Answer 5

I ignore this "rule", personally. The .NET Framework itself is full of "verb" classes: TextReader, BinaryWriter, XmlSerializer, CodeGenerator, StringEnumerator, HttpListener, TraceListener, ConfigurationManager, TypeConverter, RoleProvider... if you think that the Framework is poorly designed, then by all means, don't use names like these.

Steve's intent is understandable. If you find yourself creating dozens upon dozens of classes just to perform a specific task, it's likely a sign of an anemic domain model, where the objects that should be able to do these things by themselves are not. But at some point you have to make a choice between "pure" OOP and the SRP.

My suggestion would be this: If you find yourself creating a "verb" class that acts on a single "noun" class, think honestly about whether or not the "noun" class can perform the action by itself. But don't start creating God Objects or come up with meaningless/misleading names for the sole purpose of avoiding the verb-class.

Answer 6

Please note the use of the word "Avoid". It's not eliminate, or eradicate, or burn in hell if you ever use them.

What the author meant is that if you find yourself with a bunch of classes all named after verbs and all you happen to do is statically create thoses classes, call one function and forget about them, it probably is a sign that you are separating a little bit too much class concerns.

However, there are situation where creating classes to implement an action is a good thing such as when you have different strategies for the same action. A very good example is IComparer<>. All it does is compare two things, but there are several way of comparing things.

As the author suggested, in those cases, a good way to do that is by creating an interface and implementing it. IComparer<> comes to mind again.

Another common situation is when the action has an heavy state, such as loading files. It might become justified to encapsulate the state in a class.

Answer 7

Class names like InvoiceReader, PriceCalculator, MessageBuilder, ArticleReader, InvoiceReader are not actually verb names. They are really "noun agent-noun" class names. See agent nouns.

A verb class name would be something like Validate, Operate, Manage etc. Obviously these are better used as methods and would be quite awkward as class names.

The biggest problem with "noun agent-noun" class names is that they can give very little meaning as to what the class actually does (eg UserManager, DataProcessor etc). As a result they are more likely to be bloated and to lose internal cohesion. (See Single Responsibility Principle).

Therefore the WebService class with the IInvoiceReader and IArticleReader interfaces is probably the clearer and more meaningful OO design.

This gives you the simple, obvious noun class name "WebService", along with "noun agent-noun" interface names that clearly advertise what the WebService class can do for callers.

You could probably also give more meaning to the actual class by prefixing another noun, for example PaymentWebService.

However the interfaces are always better than a single class-name at describing more specifically what the class can do for callers. As the class grows more complex, new interfaces can also be added with meaningful names.

Answer 8

Don't follow any advice blindly. These are just guidelines.

That said, Nouns make very good class names, as long as they model logical objects. Since the "Person" class is a blueprint for all "Person" objects, calling it "Person" is very handy, because it allows you to reason like this: "I'll create a Person from the user's input, but first I need to validate it..."

Answer 9

The statement A class that has only behavior but no data is generally not really a class. is plain wrong.

Extracting behavior into a separate class is a good and common thing to do in refactoring. It can have state, but also does not need to have one. You need to have clean interfaces, and implement them anyway you find necessary.

Also, stateless classes are great for calculations you need only for a short period of time. You instantiate them (or, request a Factory of some sort to get them), do the necessary calculations, and then throw them to garbage. You can have the appropriate "version" of your behavior available anywhere, anytime.

Usually I find that different implementations of an interface have some state (set in constructor, for example), but sometimes the type of your class can determine its behavior completely.

For example:

public interface IExporter
{
    /// <summary>
    /// Transforms the specified export data into a text stream.
    /// </summary>
    /// <param name="exportData">The export data.</param>
    /// <param name="outputFile">The output file.</param>
    void Transform(IExportData exportData, string outputFile);
}

may be implemented as

class TabDelimitedExporter : IExporter { ... }
class CsvExporter : IExporter { ... }
class ExcelExporter : IExporter { ... }

To implement exporting from IExportData (whatever that may be) to a CSV file, you probably don't need any state at all. ExcelExporter, on the other hand, could have various properties for export options, but could also be stateless.

[Edit]

Moving GetInvoices and GetArticles into the WebService class means you will tie their implementation to the WebService type. Having them in separate classes will allow you to have different implementations for both invoices and articles. Overall, it seems better to have them separate.

Answer 10

Here's a classic take on 'verb vs noun' in OO:

http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom-of-nouns.html