This makes no sense to me. If the code design is bad there ought to be a concrete, unambiguous way to show this. If it's not possible to show this, how was there ever a problem in the first place?
It is possible to show in a concrete, unambiguous way, but unfortunately, the cases where the problems become clearly apparent tend to be large - way too large for a casual forum reply or a blog post.
I'll still take a stab at trying to explain what the problem is.
First, we have to define object-oriented programming, which isn't easy, because everyone seems to have their own ideas about the subject. They all do seem to agree that there should be a notion of objects, being things that have state (called "fields" or "properties") as well as associated behavior (called "methods"); further, objects form identities, that is, each object is identical to itself and only to itself, and has some sort of unique identifier (typically an ID, a memory address, or some sort of abstract handle or reference). Another feature of "proper" object-orientation is that objects are the only entities that can have state or behavior, that is, all state and all behavior must be contained in objects. And finally, while not usually completely enforced, it is considered proper style to never have anything touch an object's state directly from the outside, a concept known as encapsulation.
The core idea in OOP is to manage complexity by recursively cutting the application up into small, self-contained pieces that communicate by passing messages (or, in more modern incarnations, calling each other's methods). The promise is that by doing so, each individual object stays small and simple, thereby keeping complexity at bay.
So far, so good. The problem, however, is that objects axiomatically bundle state with behavior, that is, microstates and microbehaviors end up being tightly coupled to one another. In OOP, state does not exist without behavior (at the bare minimum, you have a constructor and a getter), and behavior does not exist without state (at the bare minimum, you have an object identity to hold the method).
This is problematic for several reasons, most notably that you give up certain useful properties of what clojure people call "unadultered data", dumb data structures that just are and don't do anything. Dumb data is easy to serialize, pass around, replicate, modify, etc., without fear of breaking any behavior, because there isn't any, there's just dumb state without any moving parts. Write it to disk, read it back, send it over the internet, nothing bad happens, in the worst case your data will be malformed, and then you won't be able to make sense of it anymore, but it's still just dumb data that doesn't do anything.
You also give up many advantages of stateless behavior: a behavior that isn't coupled to any state is automatically referentially transparent, it doesn't depend on any context, it doesn't influence its environment, all its dependencies are explicit through its inputs and outputs. We can call it on any data we like, the behavior doesn't care where it comes from, or where its outputs go to. The object-oriented world is still struggling with Dependency Injection, filling hundreds of books and thousands of blogs on the subject, when really it is nothing more than making your behavior stateless and passing ("injecting") all relevant state (the "dependencies") through inputs and outputs; as soon as you stop bundling state and behavior together into tightly coupled little bundles, the whole problem of injecting your dependencies goes away.
Let's look at an example from a domain where OOP is very popular, and which is generally regarded as a good fit for OOP: games. Our example game is a simple side scroller, where the player controls a spaceship and has to dodge incoming aliens and/or try to kill them by shooting them. An object-oriented design would start with the nouns: we need a Spaceship, some Bullets, and of course we also need Aliens. All these have a lot in common, so we should also have a base class, Entity. Here's a quick stab at the base class:
class Entity {
Entity(float x, float y): x(x), y(y)
{
}
float x; // horizontal position, center of sprite
float y; // vertical position, center of sprite
void draw(Canvas canvas);
void update();
}
And here's the alien class:
class Alien: Entity {
Alien(float x, float y): Entity(x, y)
{
}
void draw(Canvas canvas)
{
canvas.drawSprite("alien.png", this.x, this.y);
}
void update()
{
// move one pixel to the left each frame
this.x -= 1;
}
}
Bullets are just as easy:
class Bullet: Entity {
Bullet(float x, float y): Entity(x, y)
{
}
void draw(Canvas canvas)
{
canvas.drawSprite("bullet.png", this.x, this.y);
}
void update()
{
// move five pixels to the right each frame
this.x += 5;
}
}
The player ship is a bit more complex, but I'm sure you can figure it out.
And then we probably have some sort of Game object that puts it all together:
class Game {
List<Alien> aliens;
List<Bullet> bullets;
Ship playerShip;
// boring constructor left out for brevity
void update() {
for (alien in aliens) { alien.update(); }
for (bullet in bullets) { bullet.update(); }
playerShip.update();
}
void draw(Canvas canvas) {
for (alien in aliens) { alien.draw(canvas); }
for (bullet in bullets) { bullet.draw(canvas); }
playerShip.draw(canvas);
}
void run(Canvas canvas) {
while (true) {
this.update();
this.draw(canvas);
}
}
}
Awesome. We might be tempted to make canvas a field of the Game class, but the way it is now is actually a simple form of dependency injection, so I'll leave it like this.
Things are going great, so let's address the first issue: when an alien leaves the screen to the left, it should be removed from the game. There is just one problem: the alien moves itself, but how is it supposed to remove itself from the game? The alien doesn't have a reference to the Game object, so maybe we need to fix that?
class Alien {
// ...
// let's pass a reference to the game in:
void update(Game game) {
this.x -= 1;
if (this.x < 0) {
game.removeAlien(this);
}
}
}
Obviously we need to implement removeAlien in Game:
class Game {
// ...
void removeAlien(Alien alien) {
this.aliens.remove(alien);
}
}
There, problem solved. And then we use the same approach to remove bullets that are outside the viewport to the right.
Next up, shooting. The player ship needs to fire bullets when the player presses the space bar. I'll brush over the details of handling keyboard input and assume that is a solved problem; we'll just try and write the Player.fire() method. Firing a bullet is a matter of creating a new Bullet object, and inserting it into the game's bullets collection. Something like:
class Ship {
// ...
void fire(Game game) {
Bullet bullet = new Buller(this.x, this.y);
game.addBullet(bullet);
}
}
Notice that here, again, we need to pass a reference to the game into the fire method.
So right now, we have a Ship that depends on Game, and on Bullet, and a Bullet and an Alien that both depend on Game.
Next up, we want collision checks. The trouble is, we need to decide who is the active part in the collision - is it bullet.collideWith(alien), or is it alien.collideWith(bullet)? Neither makes objectively more sense than the other. And then when the collision is a fact, does the active part remove both objects, or does each object remove itself? Either way, we are introducing a dependency between Bullet and Alien, and since aliens can also collide with the player ship, we also have a dependency between those two.
In other words, all our four classes so far depend on each other. This is bad.
The alternative is to forgo the idea of making our game entities move themselves and collide themselves and delete themselves from the game and spawning new objects themselves, and delegate all this stuff to the Game object; the individual entities then just describe themselves rather than implement any actual behavior. Something like:
class Entity {
float x;
float y;
float getVX(); // get the horizontal velocity
float getVY(); // get vertical velocity
TeamID getTeam(); // whether this is a "friendly" (player/bullet) or "hostile" (aliens) entity
String getSprite(); // which sprite to use to draw this entity
}
The Ship, Bullet and Alien classes now just override the various getters to return the right values to describe themselves.
And then our Game object becomes responsible for moving all those entities and checking for collisions, something like:
class Game {
// ...
void draw(Canvas canvas) {
for (entity in entities) {
this.drawEntity(canvas, entity);
}
}
void drawEntity(Canvas canvas, Entity entity) {
canvas.drawSprite(entity.getSprite(), entity.getX(), entity.getY());
}
void update() {
for (entity in entities) {
entity.x += entity.getVX();
entity.y += entity.getVY();
if (entity.x < 0 || entity.x > maxX) {
this.entities.remove(entity);
}
else {
for (other in entities) {
if (this.entitiesCollide(entity, other)) {
this.handleCollision(entity, other);
}
}
}
}
}
bool entitiesCollide(a, b) {
if (a == b) return false; // entities never collide with themselves
if (a.getTeam() == b.getTeam()) return false; // same team, no harm done
if (/* bounding box check here, skipped for brevity */)
return true;
else
return false;
}
void handleCollision(a, b) {
this.entities.remove(a);
this.entities.remove(b);
}
}
Some stuff to observe here. First; our entity classes are kind of redundant, we can actually reduce them all to the same class:
class Entity {
float x;
float y;
float vx;
float vy;
string sprite;
TeamID team;
}
All we need now is three different functions to fill the fields with suitable data. Oh wait, we can't have free functions, this is OOP, so let's wrap them in objects:
class BulletFactory: EntityFactory {
Entity create(float x, float y) {
return new Entity(x, y, 5, 0, "bullet.png", TeamFriendly);
}
}
...and similar for ship and alien.
The second observation is that our Game class is mighty large now, well on its way to become a God Class. Let's split it up; a very logical split would be to factor out the drawing part and the updating part. Unfortunately, both parts depend on the entity list (and, in a real game, probably some other fields), so we'll introduce another class to hold the game state. So:
class GameState {
List<Entity> entities;
}
class GameDrawer {
void draw(GameState state, Canvas canvas) {
for (entity in state.entities) {
this.drawEntity(canvas, entity);
}
}
void drawEntity(Canvas canvas, Entity entity) {
canvas.drawSprite(entity.getSprite(), entity.getX(), entity.getY());
}
}
class GameUpdater {
void update(GameState state) {
for (entity in state.entities) {
entity.x += entity.getVX();
entity.y += entity.getVY();
if (entity.x < 0 || entity.x > maxX) {
state.entities.remove(entity);
}
else {
for (other in state.entities) {
if (this.entitiesCollide(entity, other)) {
this.handleCollision(state, entity, other);
}
}
}
}
}
bool entitiesCollide(a, b) {
if (a == b) return false; // entities never collide with themselves
if (a.getTeam() == b.getTeam()) return false; // same team, no harm done
if (/* bounding box check here, skipped for brevity */)
return true;
else
return false;
}
void handleCollision(GameState state, a, b) {
state.entities.remove(a);
state.entities.remove(b);
}
}
That doesn't look half bad, does it. We also need the main loop part, so let's make a class for that, too:
class GameRunner {
void run(GameState state, Canvas canvas) {
updater = new GameUpdater();
drawer = new GameDrawer();
while (true) {
updater.update(state);
drawer.draw(state, canvas);
}
}
}
Let's review what we have so far:
- class
Entity, which is pure state - class
BulletFactory, which is stateless (it has no state of its own, it just creates a return value) - class
AlienFactory, which is stateless (it has no state of its own, it just creates a return value) - class
ShipFactory, which is stateless (it has no state of its own, it just creates a return value) - class
GameState, which is pure state (a list of pure-state objects) - class
GameRenderer, which is stateless - class
GameDrawer, which is stateless - class
GameRunner, which is stateless (the only state it ever handles is two stateless objects)
Wait, weren't we supposed to be doing OOP, where state is bundled up with behavior in order to keep complexity at bay? And look what we're doing, we're writing a bunch of classes that are all either only state, or only behavior!
What if we skip the whole class business, and just write our code as structs and functions:
struct Entity {
float x;
float y;
float vx;
float vy;
string sprite;
TeamID team;
}
// our former BulletFactory class, now just a plain function; the same for
// ship and alien, left out for brevity
Entity createBullet(x, y) {
return Entity(x, y, 5, 0, "bullet.png", TeamFriendly);
}
struct Game {
List<Entity> entities;
}
void draw(Canvas canvas, Game state) {
void drawEntity(Entity entity) {
// implementation left as an exercise
// note that we're closing over the canvas here, so there is no
// need to pass it - but if you don't like that, you could still
// pass it in, it doesn't change the architecture
}
for (entity in entities) {
drawEntity(entity);
}
}
void update(Game state) {
// implementation left as an exercise
}
void run(Canvas canvas, Game state) {
// implementation left as an exercise
}
Let's look at the list of classes we had just a moment ago again:
- class
Entity - class
BulletFactory - class
AlienFactory - class
ShipFactory - class
GameState - class
GameRenderer - class
GameDrawer - class
GameRunner
And now let's compare that to the list of top-level entities we have now:
- struct
Entity - struct
Game - function
createBullet - function
createShip - function
createAlien - function
draw - function
update - function
run
The list is conceptually the same, however:
- We are no longer wrapping our behavior (functions) in useless boilerplate code to shoehorn them into objects
- We are no longer adding artificial behavior (getters/setters) to our dumb data structures to make them look more like proper objects
- Instead of a list of things that are all "objects", we now have "structs", which tells us they are just dumb data, and "functions", which tells us they are just stateless behavior. This is great news, because it makes reasoning about them much easier - we don't have to worry about behavior attached to our data, and we don't have to keep track of our functions' internal state, because there is none.
- The names of our functions and structs no longer contain words or inflections that are alien to the problem domain. Our game does not have any characters in them called "renderer", "drawer" or "runner", and it takes a lot of imagination to picture them as people. There are no factories in our game either. Yet our classes were called Factory and Renderer and Drawer and Runner. This is a sign of accidental complexity: entitites introduced not because they are present in the problem domain, but because our design paradigm needs them. By comparison, "create bullet" is a perfectly reasonable function of our game logic, and so are "draw (the game state)", "update (the game state)" and "run (the game)".
Of course this is a dramatically simplified example, and as you build more complex games, you will need to solve more complex issues than these. Most notably, some of the things that OOP gives you somewhat naturally, such as enforcing constraints on fields through setters, require special attention in such a non-OOP approach, however they can usually be solved fairly easily with a good type system, or, if you're working with a dynamic language, explicit invariant assertions baked into separate functions and used in strategic locations. Likewise, you might think that at some point, you do need polymorphic behavior, and you do - however, given a sufficiently expressive programming language, there are more than enough features for this that are more lightweight than stateful behavioral objects, such as simple first-class functions, combined with closures if extra state is needed for them.