# Circular class dependency

Is it bad design to have 2 classes which need each other?

I'm writing a small game in which I have a GameEngine class which has got a few GameState objects. To access several rendering methods, these GameState objects also need to know the GameEngine class - so it's a circular dependency.

Would you call this bad design? I am just asking, because I am not quite sure and at this time I am still able to refactor these things.

• I would be very surprised if the answer was yes. – doppelgreener May 30 '12 at 7:05
• Since there are two questions, that are logically disjoint.. the answer is yes to one of them. Why do you want to design a state that actually does something? you should have a manager/controller to check the state and perform the action.. It's a bit confusing to let a state type of object take over something else's responsibilities. If you wanna do it, C++ is your friend. – teodron May 30 '12 at 7:46
• My GameState classes are things the like the intro, the actual game, a menu, and so on. The GameEngine stores these states in a stack, so I am able to pause a state and open a menu, or play a cutscene, ... The GameState classes need to know the GameEngine, because the engine creates the window and has the render context. – shad0w May 30 '12 at 8:10
• Remember, all these rules aim for fast. Fast to run, Fast to create, Fast to maintain. Sometimes these facets are at odds with each other and you need to do a cost-benefit-analysis to decide how to proceed. If you think about it even that much, and make a gut-call, you are still doing better than 90% of developers. There is no hidden monster that is going to kill you if you do it wrong, he is more of a hidden toll-booth operator. – DampeS8N May 30 '12 at 11:11

It's not a bad design by nature, but it can easily get out of control. In fact you are using that design in your everyday codes. For example vector knows it's first iterator and iterators have pointer to their container.

Now in your case, it's much more better to have two separate classes for GameEnigne and GameState. Since basically those two are doing different things, and later you might define lots of classes which inherit GameState (like a GameState for each scene in your game). And no one can deny their need to have access to each other. Basically GameEngine is running gamestates, so it should have a pointer to them. And GameState are using resources defined in GameEngine (like rendered, physics manager, etc.).

You can't and shouldn't combine those two classes into each other either since they are doing different things by nature and combining will result in a very big class which no one likes.

So far we know that we need circular dependency in out design. There are multiple ways to create that safely:

1. As you suggested we can put a pointer of each of them into another, it's the most simple solution but might easily get out of control.
2. You can also use singleton/multiton design, with this method you should define your GameEngine class as a singleton and each of those GameStates as a multiton. Though many of developers consider both singleton and multiton to be AntiPattern I prefer this kinda design.
3. You can use global variables, hey are basically same thing as Singleton/multiton but they a slight difference in the fac that they don' limit the programmer no to create instances at will.

To conclude his answer you might use any of those three methods but you need to be careful not to over use either of them since as I said all those designs are really dangerous and might easily result in a unmaintainable code.

Is it bad design to have 2 classes which need each other?

It's a bit of a Code Smell, but one can leave with it. If that's the easier and faster way to get your game up and running, go for it. But keep that in mind because there's a good chance you'll have to refactor it at some point.

The thing with C++ is that circular dependencies won't compile so easily, so it might be a better idea to get rid of them instead of spending time fixing your compilation.

See this question on SO for a few more opinions.

Would you call [my design] bad design?

Nope, it's still better than putting everything in one class.

It's not so great, but it's actually quite close to most implementations I've seen. Usually, you'd have a manager class for game states (beware!), and a renderer class, and it's quite common that they are singletons. So the circular dependency is "hidden", but it's potentially there.

Also, as you were told in comments, it's a bit weird that game state classes perform some kind of rendering. They should just hold state information, and rendering should be handled by a renderer, or some graphic component of game objects themselves.

Now there might be the ultimate design. I'm curious to see if other answers bring one nice idea. Still, you are probably the one that can find the best design for your game.

• Why would it be a code smell? Most of objects with parent-child relationship needs to see each other. – Kikaimaru May 30 '12 at 8:34
• Because it's the easiest way to not define which class is responsible for what. It easily ends up in two classes that are so deeply coupled that adding new code could be done in either of them, so they're no longer conceptually separated. It's also an open door for spaghetti code: class A calls class B for this, but B needs that info from A, etc. So no, I wouldn't say that a child object should know about its parent. If possible, it's better if it doesn't. – Laurent Couvidou May 30 '12 at 8:43
• That's a slippery slope fallacy. Static classes can lead to bad things too but util classes are not code smell. And I really doubt that there is some "good" way to make things like Scene has SceneNodes and SceneNode has reference to Scene, so you cannot add it to two different scenes... And where would you stop? Is A required B, B requires C and C requires A a code smell? – Kikaimaru May 30 '12 at 8:51
• Indeed, this is just like static classes. Handle with care, use it only if you have to. Now I'm speaking from experience and I'm not the only one who thinks this way, but, really, this is a matter of opinion. My opinion is that in the context given by the OP, this is indeed smelly. – Laurent Couvidou May 30 '12 at 8:59
• There is no mention of that case in that wikipedia article. And you can't say its a case of "Inappropriate intimacy" until you have actually seen those classes. – Kikaimaru May 30 '12 at 10:25

It's often considered bad design to have to have 2 classes that directly refer to each other, yes. In practical terms it can be harder to follow the control flow through the code, the ownership of objects and their lifetime can be complicated, it means that neither class is reusable without the other one, it could mean that control flow should really live outside both of these classes in a third 'mediator' class, and so on.

However, it's very common for 2 objects to refer to each other, and the difference here is that usually the relationship in one direction is more abstract. For example, in a Model-View-Controller system, the View object may hold a reference to the Model object, and will know all about it, being able to access all its methods and properties so that the View can populate itself with relevant data. The Model object may hold a reference back to the View so that it can make the View update when its own data has changed. But rather than the Model having a View reference - which would make the Model dependent on the View - usually the View implements an Observable interface, often with just 1 Update() function, and the Model holds a reference to an Observable object, which may be a View. When the Model changes, it calls Update() on all its Observables, and the View implements Update() by calling back into the Model and retrieving any updated information. The benefit here is that the Model doesn't know anything about Views at all (and why should it?), and can be re-used in other situations, even those without Views.

You have a similar situation in your game. The GameEngine will normally know about GameStates. But the GameState doesn't need to know all about the GameEngine - it just needs access to certain rendering methods on the GameEngine. That should set off a little alarm in your head that says that either (a) GameEngine is trying to do too many things within one class, and/or (b) GameState doesn't need the whole game engine, just the renderable part.

Three approaches to resolving this include:

• Make GameEngine derive from a Renderable interface, and pass that reference into the GameState. If you ever refactor the rendering part out, you just have to ensure it keeps the same interface.
• Factor out the rendering part into a new Renderer object, and pass that to GameStates instead.
• Leave it as it is. Maybe at some point you'll want to access all the GameEngine functionality from GameState, after all. But do bear in mind that software that is easy to maintain and easy to extend usually requires that each class refers to as little on the outside as possible, which is why breaking things up into sub-objects and interfaces that perform a single and well-defined task is preferable.

It is generally perceived to be good practice to have high cohesion with low coupling. Here are a few links regarding coupling and cohesion.

wikipedia coupling

wikipedia cohesion

low coupling, high cohesion

stackoverflow on best practice

Having two classes reference each other is having high coupling. The google guice framework aims to achieve high cohesion with low coupling through means of dependency injection. I would suggest you read up on the topic a bit more and then make your own call given your context.