Why you should never ever ever have Managers in your class name | Hacker News Reader

Why you should never ever ever have Managers in your class name

blog.shoutt.me

36 pointsBIackSwan60 comments

Just make sure this logic doesn't leak out and pollute your whole codebase. Otherwise you will have that whole hairy mess of opening multiple async connections with auto-retries right in the middle of your business logic. Want to send a Shoutt? Well first, you need to new up a connection pool and setup the exponential back-off strategy...

You should probably consider hiding all of that complexity behind an intention-revealing interface. Something like ConnectionManager - it will manage all the hairy connection internals and let you write cleaner code as your extend your app.

Software Design is hard. :)

ThingManager is indeed a bad name, but I think the article is blurring the explanation with poorly chosen example. The example is more about wrong design than wrong name, there are certainly legitimate cases when you want a class that deals with multiple things.

ThingManager is a bad metaphor for a class that follows single responsibility principle (http://en.wikipedia.org/wiki/Single_responsibility_principle). It encourages you to stuff too much into a class because managers do many things, and it's not always clear what exactly they do - almost anything can be thought of as some sort of "management". The real reason is mentioned two paragraphs from the end - the name is simply too ambiguous.

I think there are times when you want a class to do multiple related things and encapsulating them in one place makes sense. And sometimes, the word "manager" is perfectly descriptive and sufficient.

Take the DriverManager class from the JDK (JDBC). To me, it's always been a clean way to register JDBC drivers, get connections, etc. It is responsible for internally managing available driver classes and selecting the appropriate driver to use for a particular connection string. It manages all of this internally, and, from my perspective, it is well-named.

I do think, like anything, the concept can be overused. But, I just don't agree with this kind of "never, ever use x in your class names or your code is evil" blanket statement.

Hopefully this doesn't sound too dense, but what if whole point of the class is to manage connections? What exactly is meant to take it's place?

It seems to me that the issue is that they named the class ConnectionManager when it should have been Connection. But then if you are doing connection pooling, and you need specific control over the Connection objects, then what would the harm be in delegating this to a ConnectionManager class, which manages a bunch of Connection objects?

You can create a ConnectionPool class with the relevant logic to manage Connections. If you want multiple pools (which you might connect to different servers ;) ) you just instantiate multiple ConnectionPools.

ConnectionPool makes you think of it as an actual object. ConnectionManager makes you think of this all encompassing thing that handles/manages anything that is Connection related.

You still might want a connection manager to contain your connection pools. Or even call it a messages or something, so at the relevant point in the code you just call a send function and aren't exposed to any underlying details of connections.

Specifically, what is the class doing with those pools? "Managing" is vague.

The Connection Manager in QT, for example, is not a Connection Pool. It's more of an interface to get at all the classes that represent real things within a connection process (requests, responses, active connections, etc). There is no information in the Connection Manager except system/process defaults. The Connection Manager would create connection pools if that is what's needed but it isn't Connection Pool any more than it is a connection.

That said, I sympathize with the impulse that the term "manager" says there's something wrong in your OO design. It is simply that determining how to get around the problem is hard.

The point that is being made is to make sure that the name of the class makes its responsibilities clear. The problem with the term "manager" or "handler" is that it leaves things ambiguous.

Exactly, it's description of the name of the object, rather than trying to be descriptive of what it does. AutomaticallyRetryingConnectionManager or ConnectionPool{.ConnectWithRetry,etc}

ConnectionPool makes me think of a collection of reusable connections that I can borrow from to talk to a server without the overhead of creating an object and opening a socket, it doesn't make me think of managing at all. Maybe I am doing it wrong?

I'm just hoping for a discussion, this is an important topic for me as a developer.

You got it right. Thats exactly what it should be doing. If you have additional requirements you might want to create another class for that requirement or rename it to something else.

That seems like an incredibly appropriate class name, then. I'm glad I wasn't missing something :)

You mean "pool connections" not "manage connections".

Exactly. Maybe a better name would be ConnectionFactory? This could then have multiple ConnectionPools internally for sync vs. async if needed. It's certainly better than polluting the whole code base with the logic required to instantiate an individual connection.

The counter-example given is bad in another way, if you have the max retries on a per-connection basis, you're going to be blocking trying to make connections to an end point that's down. If this state is per-end point, handled by a Manager/Factory/Pool, then you can make the retry logic global.

The main key with rules in software engineering is that it takes experience to know when it makes sense to break them. There's few things that fall in the "never ever" category.

I generally disagree with 'never ever' statements because they apply a blanket clause to a particular issue and tend to be from the perspective of someone who has experienced that problem in the past. As a relatively new programmer, this post was pretty insightful. Thanks.

Sometimes I learn better by making the mistakes and learning not to repeat them; that can be pretty annoying for others that have to work with my code I guess. Most mistakes I've made while programming are non-fatal/non-critical, but they do add up and refactoring perhaps helps with that. I think pair programming further alleviates such issues? So the lesson learnt is that I should be more mindful of how I name classes/methods, which I honestly haven't paid much attention to in the past.

There are only two hard problems in Computer Science: cache invalidation and naming things.

-- Phil Karlton

There should be a short list of words never to appear in names. "Manager" is certainly one. I nominate "Handler" as another.

From the Android SDK :)

http://developer.android.com/reference/android/os/Handler.ht...

Man, as much as I like the idea of Android, the SDK trips me up so often!

There are only 2 hard problems in computer science, caching, and naming things, and off by one errors

Unambiguous syntax is also a hard problem, it now appears.

More meaningless words:

- system. Everything is a system.

- data. Everything that isn't executable is data.

The class name modifier that irks me is "Base", as in BaseWidget or ConnectionBase. Just call them a Widget or Connection! :)

BaseWidget is useful when it's an abstract class, to differentiate it from the actual implementations WidgetA and WidgetB.

Wouldn't a "Widget" abstract class name also be differentiated from "WidgetA" and "WidgetB" implementations? Then APIs could take parameters of type Widget instead of type BaseWidget. "Widget" is a shorter, cleaner name and avoids documenting implementation details like names such as "BaseWidget" or "AbstractWidget".

There are certain ... linguistic... reasons why Base appears in class names. It's important to understand the subtlety. In certain languages, especially those with packages or namespaces, you'll likely have a namespace Foo::Bar::Widgets where you put your WhateverWidgetWhatever classes. The package foo.bar.widgets.widget, or namespace Foo::Bar::Widgets::Widget, can look weird to some; it's also longer / less predictable, so you could potentially see them simply be called Foo::Bar::Widget::Base, which avoids naming duplication, and allows developers to not have to worry about plural/singular names.

Usually, I want to make the name of my interface the cleanest possible name. In that case, my interface would be named "Widget". Then, if it makes sense, I might have a partial implementation that works for many cases - I would probably call that WidgetBase. Actual implementations of the widget would try to have a name related to what makes them different than the others - maybe Button, TextBox, etc. Those implementations might inherit from WidgetBase if it makes sense, or just implement the interface. At any rate, WidgetBase is only ever referenced by subclasses - the users of the widget code would either depend on "Button" or "TextBox" specficially, or the "Widget" interface for more generic handling. WidgetBase is really just there for convenience, and you could remove it if it turns out not to be as useful as you thought.

Java frameworks use a similar pattern with a "Widget" interface and an "AbstractWidget" partial implementation.

I personally don't like "Helper". Shouldn't every class be helpful?

Naming things is only hard if you are trying to be consistent across a large systems, or hoping that names will replace documentation and assumptions.

If you are going to ban names, I nominate the entire index of the GoF book: Factory, Singleton, Command, Proxy, etc. all sort of convey an idea of the kind of stuff they probably do, but nothing truly precise about the details.

Or I can just accept a rough meaning of the word, name my classes / modules / variables / etc reasonably and then rely on a coherent approach to design and documentation to provide with the details.

- XXXManager: owns the lifetime of objects of class XXX and the relationships between them.

- XXXHandler: responds to events relating to XXX.

- XXXFactory: creates instances of the polymorphic type XXX

- etc.

This is basic stuff.

It works fine until we realize that the app requires multiple simultaneous connections to the server. Now because you have a ConnectionManager, you go ahead and introduce an array inside the class which maintains the state of all the open connections to the server.

No competent programmer would make this mistake solely because the class is called ConnectionManager.

To me the purpose of ThingManager is perfectly clear--to manage miscellaneous or common functionality that doesn't belong in Thing. In our apps we call it ThingService, and I'm sure other groups have a different convention.

For example, we're using a legacy codebase now where there is some complicated logic involved in managing attributes for products. ProductService is a singleton with no internal state that does things like removeAttribute(product, "foo") and serves other purposes. Before we introduced this pattern this kind of functionality was copy-pasted all around our controllers.

In short, I don't see the harm in this convention. Perhaps Manager or Service is bland name, but it's immediately clear to our developers what it does, and if you want to see if something has been done before with products, you look in Product or ProductService.

> the purpose of ThingManager is perfectly clear--to manage miscellaneous or common functionality

Ironic, isn't it? :)

PS. Don't get me wrong - a Manager, or Util, or Helper is not the greatest crime against software design. The intention here is clear, functionality - no, not really. However engineering is about tradeoffs, and it is sometimes practical to have a dumping place for miscellaneous stuff. Just keep in mind that it is a "be careful" territory, those places do tend to turn into ugly and hairy mess over time.

Yeah, Manager is a "code smell", but not a showstopper.

Good workers don't need much managers.

It's not that it "doesn't belong in Thing", but rather, it can be performed by another entity. These are the same fundamental "responsibility" questions we struggle with in designing OO systems every day. When you have two or more entities collaborating on a responsibility, which one of them get the actual method? Incidentally, this is kind of what real life managers are for too, as much as we like to dislike them.

The problem is in OO, or rather how people learn OO.

They learn that objects encapsulate behavior, and soon start deriving all kinds of non-objects to hold their actions ("ConnectionManager", "ConnectionHandler", etc.), while the state is kept all over the place.

You should look at objects as just another data structure. Once you realize this, it's pretty clear that you should have a "Connection" that keeps the state and a bunch of methods to manipulate it. This comes natural if you program in C or other paradigms, where a "Connection" would be a struct or something like a hash.

Another code-smell is a codebase littered with singletons.

>* They learn that objects encapsulate behavior, and soon start deriving all kinds of non-objects to hold their actions*

If I understand you correctly, it seems you would have reached the opposite conclusion: that is, if I learn that objects encapsulate behaviors/actions then I would create classes like "Connection" with all the attendant methods and would have no need for external classes like ConnectionManager.

But, if I treat a Connection like a struct, then I would need these external classes like ConnectionManager to perform the actions.

So, it seems exactly opposite from what you suggest.

> So, it seems exactly opposite from what you suggest.

Can I make it more clear? People learn (wrongfully) in OO that objects encapsulate behavior, so the first thing they do is create "Manager" classes and outline their program flow. This is even worse in languages that only gives you classes to work with - e.g., Java. Wrong. The first thing one should do is define the data structures - and when using OO, doubly so, because the data structures will map to classes seamlessly.

When you have those "Manager/Controller/Handler" classes, it's a pretty big sign you're inventing a non-object to store all the control flow in one place, instead of thinking about proper objects and message passing.

That is clearer. Thanks. Now, I know that it makes no sense. :)

Classes can and should encapsulate behavior in good OO design. This is why classes can contain methods. I don't know if you think that feature is also "wrong" in an OO language, but without it some of the primary benefits of OO, such as polymorphism, begin to disappear.

Speaking of polymorphism, how do you propose that would be achieved under the constraints you would impose? Prior to editing the parent, you had written that you prefer something like:

Connection conn = Connection( params )

instead of

Connection conn = new ConnectionManager().getConnection( params )

In that case, how would you propose that the former code (i.e. your preferred approach) instantiate the appropriate Connection subclass? There wasn't even a new operator or valid construction syntax in your preferred approach, so perhaps you recognized this limitation and were suggesting some magic would occur.

Before editing, you also said not to "get you started on statics". So, without statics, you couldn't even do something like Connection.getInstance(params) to allow the Connection class to determine which subclass to create. So, the calling code would have to be aware of which subclass to instantiate, which would be simply awful.

Perhaps this is why you edited your comment.

But leaving your edits aside, you still seem to think of objects as only data or structs with no behaviors, which is an extremely limiting constraint on OO design. In fact, it would eliminate so many benefits of OO programming that it wouldn't be OO at all.

> Classes can and should encapsulate behavior in good OO design. This is why classes can contain methods. I don't know if you think that feature is also "wrong" in an OO language, but without it some of the primary benefits of OO, such as polymorphism, begin to disappear.

Of course, classes encapsulating behavior is the reason behind OO. What's wrong is the backwards thinking: "I need a place to hold my behavior, let me write a ThingManager class for that".

> Connection conn = Connection( params ) instead of Connection conn = new ConnectionManager().getConnection( params ) In that case, how would you propose that the former code (i.e. your preferred approach) instantiate the appropriate Connection subclass? There wasn't even a new operator or valid construction syntax in your preferred approach, so perhaps you recognized this limitation and were suggesting some magic would occur.

The point is, why do you have a class after all then? A normal function that returns the appropriate object suffices. Of course this is not possible in pure-OO languages, like Java, so you abuse classes to hold logic like a namespace. Static methods are an even worse perversion of OO, for the same reason (lack of pure functions inside namespaces).

>The point is, why do you have a class after all then? A normal function that returns the appropriate object suffices. Of course this is not possible in pure-OO languages, like Java, so you abuse classes to hold logic like a namespace. Static methods are an even worse perversion of OO, for the same reason (lack of pure functions inside namespaces).

But this implies that there is never a natural hierarchy amongst classes or a case wherein you want a collection of objects to be managed by a class whose responsibility is to know about or manage objects. Those situations can and do arise. And shoving such functionality down into the "managed" objects themselves seems less desirable in my view. In such a design, the managed objects are basically saying "I am a subtype of x, and I also know about and manage other subtypes of x, similar to me".

Why should each x carry around such baggage and worry about managing other objects? To me, that breaks good OO design and separation of concerns. So, while I think Manager concepts can be abused, I also believe it is overkill to simply "outlaw" their use in any situation.

Again, going back to the DriverManager example, I would say that such a class is a more natural fit for managing drivers, registering them, etc., than the Driver class itself.

I agree that static methods aren't very OO-compliant. Ironically, I think they exist in pure OO languages specifically because of the strong class orientation. So, they are at times, in effect, the "normal function that returns the appropriate object subclasses" that you prefer.

Instead of saying "don't do X", wouldn't it be easier to show a bunch of examples and let people map it onto their own problem space? Here, I'll start. These are some class names in my big depot here. They're not perfect, but what is?

Dep: from my build tool, it's a dependency, like when you do #include "foo/bar.h", then foo/bar.o is the dep, and it is built from foo/bar.cc and foo/bar.h. It figures out the inputs and outputs and whether it needs to be rebuilt, and can do the building, too.

DepTracker: it tracks all of the Deps. It's responsible for creating them as necessary and making sure they're all built (compiled) and ultimately linked if building a binary.

HttpClient: uses libcurl, generates GETs and POSTs and the like, for tasks like fetching feeds (Atom/RSS), talking to Mozilla's Persona (browserid), and Stripe.

HttpServer: uses libmicrohttpd, and gives the program an embedded HTTP server. This is great for debugging info.

FormatPost: takes my flat text in and gives back formatted HTML for my "/w/" posts, Atom feed, or even the books.

MySQLClient, PGSQLClient: interfaces to those external libraries as you would assume.

ParseAtom, ParseRSS: turns different flavors of XML into something a little more reasonable for my own purposes.

TCPSource, USRPSource, OsmoSDRSource: for different kinds of GNU Radio input scenarios.

These are just a few of the larger set. The word "Manager" does not appear in any of them, but that wasn't a conscious decision. It just hasn't happened.

People use words (terms/names) to communicate. I am not sure if most developers think deeply about all this -- nor do they care, or want to care. My observation is that developers just do and mimic what they find commonly used in their environment. Very much like we learn and use English. So, if you are a Java developer and you take the API as your template -- then that is the vocabulary that seeps out.

It makes me feel pretty dense to admit this, but I don't see the connection (ha) between this basic good advice (use short, precise, descriptive class names -- certain classes of names are a warning sign) and the famous "Kingdom of Nouns" essay the author is so eager to have us read. Because a "ThingManager" is still noun-style all the way, even if you rename it to "Thing."

The "Kingdom of Nouns" essay is about the forceful enforcement of Nouns even when they aren't entirely necessary. As a consequence of this thinking, it has led to the rise of use of -Manager classes even when not required.

More importantly, the acceptance in the developer community that its ok to define large number of classes with that name is just a bad influence. Noun style is not a bad thing at all. Its just that when you think of everything as Nouns that things go downhill (which lead to naming things -Managers, -Handlers etc).

...when you think of everything as Nouns that things go downhill (which lead to naming things -Managers, -Handlers etc).

Yeah, not seeing it. If I "think of everything as Nouns" then I might go with "Noun" or "NounManager" as a name - but they're both nouns, and I don't see how pointing that out helps.

I completely agree with the point that "Manager" style classes are a warning sign that something could be usefully refactored. But if you're just going to refactor stuff into other nouns, and you're trapped in a noun-centric language anyway, I don't see any real connection between the advice on the one hand and the message about how trapped I am on the other.

Class names are pretty much meaningless to me. It's hard to name classes, so in my experience, I tend to look past them and understand the responsibilities of the class and pretty much ignore the name. If it's too hard to understand the class responsibilities without a lot of work, then the class is definitely poorly designed, most likely too complicated.

The problem is when someone is trying to browse the code. When things are well named, then it is much easier for someone unfamiliar with the code base to find their way around. I've done a fair amount of hacking around in open source projects. There are some projects where I can download the source code, know what I want to do going into it, and just do it. In other projects, when I am at a place I think is relevant to what I want to do, I have to dive deep into the programs abstraction layers just to see how certain objects and functions behave and what they do. Some of that is likely an unavoidable result of me needing to get to a rather core part of the code, but it seems like a lot of it has to do with the clarity of naming.

If I need to know the precise behavior, I will generally look it up anyway, but I do not need to know the precise behavior of everything.

I fail to see the problem of using manager in the name, what I see is a class that should have been renamed to connection extended etc if needed, and multiple instances created, then if needed, a ConnectionManager created to take care of them.

The problem isn't the name but the person who instead of renaming it went with a monolithic approach.

I think the more salient point here should actually be if you have a thingManager, you should almost certainly have a thing.

From a external perspective, using a thingManager is likelier to be easier, but from an internal perspective, having both a thing and a thingManager will force you to think about putting the logic in the correct place.

Be that as it may, but it should also be easy to change the name later on (perhaps to amend your original mistake and refactor to a better structure and name).

If your tools don't let you change class names easily, they have room for improvement.

There should be similar guidelines for things like database tables. I've seen too many tables with names like "foo_data", "foo_info(s)", or even "foo_details" (in a one-to-many relationship from "foos").

TL;DR: Don't name a class for a <thing>, <thing>Manager.

I've heard it said that having any class name as [verb]-er is an OOP anti-pattern, as your objects shouldn't be simple nounifications of verbs.

That rules of thumb is arbitrary attempt to make code more like fancy English.

Why is "connection" OK but "connecter" bad?

It's not the name per se that is the problem; the point is that an "-er" class is indicative that you're actually using a procedural rather than OO paradigm, and thus attempting to arbitrarily box up a bunch of actions into an object.

Neither connection nor connector would be good, under this view, if it's just a thin package around an imperative program ("COBOL written in Java").

I would not follow the rule strictly if I find that people working with me understand that a class with name XXXManager more than a class called XXX.

Whew, good thing mine is called ConnectionController.

My code has ManagerControllers everywhere!

Don't give that coder a Manager class. Coders hate managers.

Reply on news.ycombinator.com