Friday, April 09, 2010

Am I a Bad Man for Naming my Classes after Patterns?

A couple of weeks ago a rather innocuous tweet from Nat Pryce (@natpryce) flashed on my screen which read:
“Pattern names should be obscene so that programmers will not be allowed to name classes after patterns.”
It was quickly picked up by the tweet-o-sphere as programmers from all over echoed the chant. But wait … I name my classes after patterns?! Was I committing some sort of programming faux pas? For years, in discussions with other developers over design issues or implementation details, I would always try to reference the appropriate pattern where applicable. And these Pattern-based conversations inevitably made their way back into the code.
“That server uses a Reactor Pattern so it's not going to perform as well on a multi-core machine. Perhaps you should consider one based on Leader-Follower?”
“Each instance of that class is going to occupy a lot of memory, but we need to cache the most popular ones. Would the Evictor Pattern be the best choice here?”
If you go back and look at the original GOF book the rationale given for Patterns was based on a quote by Christopher Alexander who, speaking of buildings and towns, said,
"Each pattern describes a problem which occurs over and over again in our environment, and then describes the core of the solution to that problem, in such a way that you can use this solution a million times over, without ever doing it the same way twice"
Speaking in Patterns does something very important between programmers … it raises the level of the conversation. If I talk to another developer about a Strategy Pattern, I don't need to say “Create an interface and move the specific implementation for each way to lay out the nodes into each derivation of it. That way, if we can't lay out the nodes one way, we can just change that class and try another way. Get it?” If my colleague knows what a Strategy Pattern is, he'll just say “Yup, good idea.” (or "Are you insane?")

I love whiteboards, but I don't need to write on them every day. That's where we would be without Patterns.

When I name my classes with a Pattern name, I'm doing the same thing, I'm telling you how I'm attempting to solve a problem without requiring you to look into the nasty implementation details. A comment on a class or interface definition is not the same thing. It's not going to follow everywhere the class or interface is referenced so the details get lost.

If I see a package that has three classes: FooStrategyFactory, FooStrategy and FooState I have a very good idea what's going on in that module. I don't need to look at the code.

This was the basis for my discussion with Nat when I asked “Why is this a bad thing?” We chatted back and forth (as best you can on Twitter) about my position and his first objection was “Why do you need to call it FooStrategy? If there is an Interface you can assume there are going to be more than one of them.”

A good point. Perhaps there is something to his argument, consider this developer conversation:

“Oh, there are going to be several different ways to lay out those nodes in the graph?”
“Yes, I'm going to switch between then using a Strategy Pattern.”

And I set off to coding. I start with a LayoutStrategy interface and move down into the implementation definitions I may create concrete classes like LinearLayout, CircularLayout, SquareLayout. I will concede that you don't need to repeat the pattern name in these derived classes. There is no need to call them LinearLayoutStrategy, CircularLayoutStrategy, etc. because you'll see that in the class definition:
interface LayoutStrategy ...
class LinearLayout implements LayoutStrategy …
And, in this scenario, you probably never should see these derived class names anyway. They are likely being instantiated from a Factory or being “injected” into some other class. A common calling scenario might look something like:
LayoutStrategy layout = layout_strategy_factory.get_layout_strategy(“circular”);
renderer = new GraphRenderer(layout);
And, arguably, it's a little verbose. But, how does it look without the Pattern name in the class?
Layout layout = layout_strategy_factory.get_layout_strategy(“circular”);
renderer = new GraphRenderer(layout);
That looks pretty vague to me. I can't infer how to interact with the class based on the name like I can with LayoutStrategy. How about we put the pattern in the instance variable?
Layout layout_strategy = layout_strategy_factory.get_layout_strategy(“circular”);
renderer = new GraphRenderer(layout_strategy);
Uggh! Now the developer has to know what the implementation approach is in order to correctly name the local variable. No way.

Personally I think the first snippet offers the most value. But I find all of the above snippets equally useless. If I was a first time reader of that code, I wouldn't know if Layout objects should simply position the nodes or render them as well? What is the purpose of each class? I have a hint about their purpose and I have a good idea what's under the hood, but I have no idea about the separation of duties.

And this seems to be one of Nat's concerns as well. Some other interesting points he mentioned related to this were:

  1. that classes should be named after their purpose and not their implementation, and
  2. that every class applies and/or plays roles in many overlapping patterns. Picking just one for the name ignores all others.

But I'm not sure I agree. I'm a big fan of the Single Responsibility Principle. That a class should do one thing and one thing only. If a class is implementing more than one Pattern it should be a Composite of the separate functions. Perhaps his concern is that some programmers name a class after a Pattern and then the class ends up doing something different? Bait-and-Switch naming is a viable concern. But that's something that should be caught in a code review and not a reason for a blanket coding style guide such as “Don't Name Classes After Patterns”.

Class Naming becomes an issue for two different groups of developers:

  1. Consumers of the classes
  2. Maintainers of the classes

And most frequently, these are different groups with different expectations from the class name. Knowing the Pattern a class follows helps both groups. If it's a Strategy, I'm going to look for a place where work occurs. If it's a Leader-Follower, I'm going to look for the event handler and the thread dispatcher. If it's a State object, I'm going to try and find the Finite State Machine and methods for transitioning between states. Those are important clues when you have to look at legacy code or use an API for the first time. I don't even need to read the docs to know what I'm looking for.

So, the problem seems to be this:
If the classes are granular enough, naming them after Patterns is useful since it provides an indication of Behavior. But it is not expressive enough to convey Purpose (in the context of the problem).
How can we name a class so as to convey behavior and purpose? I'm certain the wrong way would be to give classes names like LayoutButDoNotRenderStrategy and RenderNodesButDoNotPositionThem.

But if the class is named after the behavior, perhaps the package/module names should convey purpose and context? The package name describes what the Factory, Strategy and State objects are acting on.

In our toy example, we need the the GraphRenderer class to live in a package that screams “I draw stuff!” and the Layout classes live in a package that screams “I position stuff!” Perhaps the following works:
package Graph.Node.Placement;
class Factory;
interface Strategy;
and
package Graph.Rendering;
class Renderer;
Our calling code now looks like:
import Graph.Node.Placement.Factory;
import Graph.Node.Placement.Strategy;
import Graph.Rendering.Renderer;

Factory strategies = new Factory(...);
Strategy layout = strategies.get_layout_strategy(“circular”);
renderer = new Renderer(layout);
Personally I find this more than a little sterile but immensely readable. If you want to make it like a hammer-to-the-head you could do:
Factory strategies = new Graph.Node.Placement.Factory(...);
Strategy layout = strategies.get_layout_strategy(“circular”);
renderer = new Graph.Rendering.Renderer(layout);
Works for me. Am I still a bad man for my rationale? What am I missing here? What are some viable alternatives? What works for you? It would be interesting to try this experiment with other Patterns, but I think single implementation Patterns are the easy cases. Overall, it's a tricky topic and class/package naming is more Art than Science.

Finally, some side notes:

  • In order for any of this to work, your team needs to agree on the reference sources for their Patterns. If people start pulling Pattern names out of thin air the conversation doesn't benefit. For example, you could state that your project can only reference patterns from the GOF, Pattern-Oriented Software Architecture Vol 2 and Core Security Patterns. At least then the developers have somewhere definitive to start their searches. (see my previous article on Coding Standards)
  • We do agree that calling things Manager is bad. But this is a simple problem to solve since there is no Manager Pattern. Did the developer mean Factory? Calling a class a Manager is a definite code smell and should be avoided.
  • Having Python as my weapon of choice for the last ten years it kills me to have to make all these verbose module definitions. But for projects of any meaningful size you need to be regimented in your code organization.

Look forward to your feedback in the comments!

5 comments:

Adam Burry said...

I'm neither for nor against using pattern names in class names. Perhaps I do not have enough experience.

What I *am* against is using non-standard names.

One of the reasons I hate working on MS-Windows is that nothing is ever named the way I would expect. It makes it very hard to find information when you don't speak the local language.

Today I found a good example. This was not in the MS docs, but it was in a Windows app, so I'm blaming MS anyway, somehow their thinking infected this hapless programmer.

The offense was using the word "nap" in place of the word "sleep". It is not a nap! And it was not a case of namespace collision or anything like that, it was just a random act of evil.

So, use pattern names in your class names if you like, but as you point out in your article, USE THE RIGHT FREAKIN' NAMES!

Kyzer said...

You may or may not be a bad man.

There are bad men out there who are so interested in appearing cutting-edge, they will take every opportunity to use jargon, even in their class names. Their purpose is not to communicate clearly, but to peacock themselves and say "hey everyone! I know software architecture!"

The popular kids these days are calling those types "Architecture Astronauts".

MyDarkSecret said...

@adam: Agree. I've had an issue with Dependency Injection, as a term, for a while now.

@Kyzer: Certainly to use a pattern name to sound superior is lame. If the conversation is not two-way, where both parties understand what is being said, you've lost the point of using Patterns in the first place.

But it does bring up an interesting issue, would you say that the use of patterns as a communication tool is a dying art? When you look around your team, do they speak with patterns or is that a cryptic dead language?

PS> Not sure this is really related to the point of Joel's article (architecting into the abstract), but I get your point.

Maxim Khailo said...

Except that the GOF patterns aren't really patterns in the Christopher Alexander sense.

Dave said...

I have always said that packaging, determining the appropriate name space, is one of the hardest parts of programming and one of the least addressed topics in programming discussions. I agree that the name space provides the context. I *almost* feel that imports should be left out.

David