views:

110

answers:

3

In my app I have a Controller that's started by the main method. The controller initializes hooks, database connections, the UI, another connection, and other things. It holds most of the state of the program (no, its not a Singleton). In another example there's a controller for the bot that handles interpreting and sending out of commands. Both are quite large files.

I've read up on God objects, but I don't really know of a way to split it up. If I split up the interpreter and dispatcher in the bot, it's going to make a horrible call chain (something like getBot().getParser().getOutput().sendMessage(recipient, message)). Similarly there in the first Controller if I split things up you would just have Data objects that hold fields and some alias utility methods. Splitting them up would just make things worse. And before you assume that its unmaintainable, it actually isn't. I didn't even write the Bot controller but I still know whats going on.

The issue though is that the Bot class is 2000 lines long (probably shorter if I took out the Javadoc comments) and the Bot is roughly 1000 lines long. Lots of lines = God object. But is it okay for one or two core classes of a project?

+3  A: 

If you are uncomfortable with the size and complexity of a class, then it's usually a good indicator that a better design could be done. But don't measure just on size. If a class is easy to understand and follow, but contains a lot of code, that doesn't necessarily mean it's a candidate for re-factoring. I've seen people get carried away with this and the mess they create in the pursuit of making things small was way worse that the original code. On the other hand, I've read through classes from end to end several times and still had no idea what they do.

The question I'd be asking is - If I gave this to another developer, would they be able to easily understand and maintain it?

If the answer is yes, then the odds are that you don't need to do anything. If not, then a re-factoring is in order.

With reference to god objects, reading your post, it does sound like this class is doing too much. I'm wondering if firstly you could re-factor out the state into a set of model objects as a starting point. Then your class starts to look more like a configuration factory of some sort.

Derek Clarkson
+3  A: 

"Lots of lines" doesn't mean that the class is a god object at all, that's a terrible terrible benchmark for figuring out whether or not you should refactor something. Some things are very complicated and warrant a complicated and inherently large object. The idea of a God object is what the class does.

For example if I made an object that could

DoMyTaxes()
GiveMeHugs()
LogThisError()
StartGameLoop()

The object would qualify as a god object, even if it may only be 100 lines of code. The basic idea is that all of the above are completely unrelated (in the business logic end of the spectrum) so why in the world would they all be part of the same object. If I decided to make hugs last longer I could end up screwing up my taxes. Enter the IRS.

However, if you are working on a physics simulator, lets say, and the Classical() class would have methods/objects such as:

Space()
Time()
Velocity()
Speed()
Mass()
Acceleration()
Gravity()
Force()
Impulse()
Torque()
Momentum()
AngularMomentum()
Inertia()
MomentOfInertia()
ReferenceFrame()
Energy()
KineticEnergy()
PotentialEnergy()
MechanicalWork()
VirtualWork()
DAlembertsPrinciple()

(courtesy of Wikipedia)

This object would not be a god object. It's a complex object. Everything that deals with Newtonian physics goes through it, but it's not a God object.. it's just a really really big object. The above could end up being thousands of lines of code.

The Quantum() object would be even more complex, needless to say.

To reiterate, the idea is about a program's behavior, not data flow:

you don't care whether a single object holds a lot of the app's data, or whether most flows have to go through a single object. What has more impact on maintenability is when a single God Class(tm) holds too much behavior (business code).

If you think there's a problem, you could try to implement different forms of mediation, or uglier patterns such as dependency injection.

David Titarenco
Good answer. Following that principle I guess my object is just complex since it handles both parsing, output, and hooks (although hooks are moving out). Thanks for the answer
TheLQ
+2  A: 

I would suggest that the physics/movement engine should definitely be separate from the language interpreter; although the language interpreter will need access to some of the public methods and properties of the physics engine, there's no reason the two aspects of the robot should be in the same class. The language interpreter itself could be subdivided into a few classes, as should the movement engine. There may be a master-control object, but it should have a relatively small amount of code. It, the main movement engine, and the main language engine, should all delegate most of their work to the objects that comprise them.

supercat