Skip to content

On detecting God Classes

Opinion on Twitter was divided when I suggested the following static analyser behaviour: report on any class that conforms to too many protocols.

Firstly, a warning: “too many” is highly contextual. Almost all objects implement NSObject and you couldn’t do much without it, so it gets a bye. Other protocols, like NSCoding and NSCopying, are little bits of functionality that don’t really dilute a class by their presence. It’s probably harmless for a class to implement those in addition to other protocols. Still others are so commonly implemented together (like UITableViewDataSource and UITableViewDelegate, or WebView‘s four delegate protocols) that they probably shouldn’t independently count against a class’s “protocol weight”. On the other hand, a class that conforms to UITableViewDelegate, UIAlertViewDelegate and MKMapViewDelegate might be trying to do too much – of which more after the next paragraph.

Secondly, a truism: the goal of a static analyser is to ignore code that the developer’s happy with, and to warn about code the developer isn’t happy with. If your coding style permits a class to conform to any number of protocols, and you’re happy with that, then you shouldn’t implement this analyser rule. If you would be happy with a maximum of 2, 4, or 1,024 protocols, you would benefit from a rule with that number. As I said in my NSConf MINI talk, the analyser’s results are not as cleanly definable as compiler errors (which indicate code that doesn’t conform to the language definition) or warnings (which indicate code that is very probably incorrect). The analyser is more of a code style and API use checker. Conforming to protocols is use of the API that can be checked.

OK, let’s get on with it. A class that conforms to too many protocols has too many reponsibilities – it is a “God Class”. Now how can this come about? A developer who has heard about model-view-controller (MVC) will try to divide classes into three high-level groups, like this:

MVC high-level architecture

The problem comes when the developer fails to take that diagram for what it is: a 50,000-foot overview of the MVC architecture. Many Mac and iOS developers will use Core Data, and will end up with a model composed of multiple different entities. If a particular piece of the app’s workflow needs to operate on multiple entities, they may break that out into “business logic” objects that can be called by the controller. Almost all Mac and iOS developers use the standard controls and views, meaning they have no choice but to break up the view into multiple objects.

But where does that leave the controller? Without any motivation to divide things up, everything else is stuffed into a single object (likely an NSDocument or UIViewController subclass). This is bad. What happens if you need to display the same table in two different places? You have to go through that class, picking out the bits that are relevant, finding out where they’re tied to the bits that aren’t and untangling them. Ugh.

Cocoa developers will probably already be using multiple controller objects if they’re using Cocoa Bindings. Each NSArrayController or similar receives its object or objects, usually from the main controller object, and then gets on with the job of marshalling the interaction between the bound view and the observed model objects. So, if we take the proposed changes so far, our MVC diagram looks like this:

MVC - Slightly closer look

The point of my protocol-checking code is to go the remaining distance, and abstract out the other data sources into their own objects. What we’re left with is a controller that looks after the view’s use case, ensuring that logic actions take place when they ought, that steps in the workflow only become available when their preconditions are met, and so on. Everything related to performing the logic is down in those dynamic model objects, and everything to do with data presentation is in its own controller objects. Well, maybe not everything – a button doesn’t exactly have a complicated API. But if you need a table of employees for this view controller and a table of employees for that view controller, you just take the same table datasource object in both places. You don’t have two different datasource implementations in two view controllers (or even the same one pasted twice). This makes the diagram look like this:

MVC - more separation

So to summarise, a class that conforms to too many protocols probably has too many responsibilities. The usual reason for this is that controller objects take on responsibility for managing workflow, providing data to views and handling delegate responsibilities for the views. This leads to code that is not reusable except through the disdainful medium of copy-pasting, as it is harder to define a clean interface between these various concerns. By producing a tool that reports on the existence of such God classes, developers can be alerted to their presence and take steps to fix them.