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.
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:
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.
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:
It is generally perceived to be good practice to have high cohesion with low coupling. Here are a few links regarding coupling and 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.
