The Single Responsibility Principle (SRP) doesn't strictly mean that mean that no class should ever do more than one thing. A slightly more accurate definition would be that adhering to the SRP means that one class expresses one single concept, and as much as is reasonable, utilizes other classes and interfaces to express its workings, so as not to be bound to the low-level details of such external systems.

So, taking your example, its perfectly reasonable, under the SRP, to have an entity which holds all those things that you mentioned, as long as those which are not primitive types are reasonably encapsulated, and themselves hold to the SRP. In other words, an Entity which is made up of a Position class, a Mesh class, a Skeleton class, and primitive data members representing health and ammo, still expresses the singular concept you call "entity". When an instance of the Entity class is made to change its position, it uses the interface of its position member to affect that change in itself; the same goes for functions related to its skeleton and mesh members. Note that this is a reasonable choice, but far from the only one (in fact, I'd probably make health and ammo distinct classes, as they have behave differently than their underlying primitive types), and also that the Entity class might make itself up through a combination of composition, reference, inheritance, or component-entity systems.

Tispe, on 07 Jun 2013 - 20:44, said:
Lets take an "Entity class".
members:
-Position
-Health /* Wrong */
-Ammo /* Wrong */
-Skeleton /* Wrong */
-Mesh /* Wrong */

methods:
-Move() /* Somewhat less wrong, indirectly */
-Shoot() /* Wrong */
-TakeDamage() /* Wrong */

Why would a building have health, be able to shoot, or take damage?
Why would a skybox have a skeleton?

You won’t understand what the single-responsibility principal means unless you understand what it means to have a single responsibility.

A CEntity is the lowest-level thing that gets into your game world. Every single object inherits from it. It has 1 job and 1 job only: Provide things with a parent/child relationship.

Next level up you have a CActor, which provides things with world/local orientations.

I didn’t mark your suggestion that an entity have a position wrong because an entity and actor are often combined into 1 and the line between them is ambiguous.


So the only thing every single object in your game world has in common is that they can be parented under each other and they have a local position, which gets translated into a world position whenever changed.
How could you give a health bar to every single object in your game? Does the ground have health? Do buildings explode if you punch them enough?
At this level, we haven’t even gotten to models yet.

CModelInstance -> CActor -> CEntity

Now we have an actual mesh, which can represent some physical objects in the scene.

CDrawableModelInstance -> CModelInstance -> CActor -> CEntity

Now we have physical objects that can be drawn. Remember, not every object is visible. Invisible walls obstruct your path just as well as any other objects do.


CPlayer -> CDestructable -> CDrawableModelInstance -> CModelInstance -> CActor -> CEntity
CGenerator -> CDestructable -> CDrawableModelInstance -> CModelInstance -> CActor -> CEntity
CGiantMoth -> CDestructable -> CDrawableModelInstance -> CModelInstance -> CActor -> CEntity

Now we have something that can take damage. And it took 6 classes to get there.
This is Single-Responsibility Principal. It means a class should do only when it needs to do. It may do more than one thing, of course, as long as it makes sense for it to do that. A CModelInstance can be animated and thus have a skeleton. Because we all know that things may move even if we can’t see them. It makes sense for animation to be part of CModelInstance rather than CDrawableModelInstance. And this is a balance between “is a” and “has a”.

This is Single-Responsibility Principal. Things may have more than one duty, but those duties are logically a part of the object, and may even be delegated to a member of that object. For example a CModelInstance introduces the concept of animation to an actor, but a CSkeleton is the class that actually performs everything related to animations.
Likewise, an actor does not handle the responsibility of moving just because it introduces the concept of a position into the mix. The position, scale, and rotation of anything in the world is handled by the COrientation class. A CActor may introduce local positions to objects, but managing those positions is not its responsibility—that falls to COrientation.

This is Single-Responsibility Principal.


L. Spiro

Inheritance is just a tool that you use to express a particular kind of relationship between classes. Its neither indicative of, or necessary for, SRP to be maintained. There are many, many ways to express relationships and compose objects in C++.

SRP is really about how complex concepts are broken down into their most atomic parts -- the idea is that its much harder to understand and reason about a large, complex ball of code and data than it is to understand and reason about a functionally identical system that's made up of smaller, self-contained portions of that functionality. Additionally, because those smaller systems are now independent of one another, you can use them as the basis to build other complex objects easily.

Sorry for the bad naming, but when I think of an Entity I think of a living creature like a PC or NPC, which has a mesh, skeleton etc.

It seemed so obvious that SRP ment that you made classes that contained data and then you made classes that manipulate that data, and then a render class that draw that data. Why a PC and a Sound clip needs the same base class is not clear to me.

Entity usually refers to any interactive element within the game world, or something along those lines. PC/NPC could be classified as 'Actors'. The is-a relationship would be, "Player is-a Actor is-a Entity", so Actor could inherit from Entity and Player could inherit from Actor. Inheritance is its own bucket of worms, and should be studied separately and comprehensively in order to save you a lot of grief later on.

SRP does not mean that you have classes that are all data and classes that are all action. That's really not a great concept to bind yourself to either, as it sounds like a recipe for inappropriate intimacy and eventually a god class. Additionally, SRP does not apply only to classes. It also applies to functions, modules, and really any segregable part of a program.

Make simple parts, then use them to build other simple parts.

The real goal is that someone can look at one of your parts and easily understand what it does just by reading through it, without needing to look up how its component parts work, or understand how it fits into the larger system.

Now we have physical objects that can be drawn. Remember, not every object is visible. Invisible walls obstruct your path just as well as any other objects do.
CPlayer -> CDestructable -> CDrawableModelInstance -> CModelInstance -> CActor -> CEntity
CGenerator -> CDestructable -> CDrawableModelInstance -> CModelInstance -> CActor -> CEntity
CGiantMoth -> CDestructable -> CDrawableModelInstance -> CModelInstance -> CActor -> CEntity

I didn't know inheritance and SRP were so close. Can't you have SRP without inheritance?

Yes, IMHO, the above is an abuse of inheritance. It's using "is a" (inheritance) where "has a" (composition) would suffice. Standard OO doctorine is to default to composition and only use inheritance where it's actually required -- i.e. where you actually need to polymorphically operate on some abstract type through an interface -- so if ModelInstance isn't implementing an interface that it's inherited from Entity, then it shouldn't inherit from it.

The main symptom that appears from defaulting to inheritance over composition is that your code is much more rigid and less reusable. e.g. what if we want to have an invisible wall that is destructable? Destructable inherits from Drawable, which the wall is not, so now we need to duplicate some code an make an InvisibleDestructable...

With composition and the SRP, the Player class has the single responsibility of acting as the glue that makes all it's member components work together:

struct Entity { virtual void Update() = 0; virtual ~Entity(){} };//Entity is often something like this
struct Player : public Entity
{
  Destructable  health;
  Model         visual;
  RigidBody     physical;
  Transform     position;
  AmmoContainer inventory;
//implement entity interface:
  void Update() {...}
};

IMHO, I'd get rid of the Entity base class in the above example as well, as it's too broad of an abstraction...

Instead of using inheritance to implicitly link together all of the individual components, in a more flexible design you can create the inter-component bindings when you construct the composite object:

struct Player
{
  Player( PhysicsWorld& pw, VisualScene& vs )
  {
    health.SetOnDamage( bind(&Player::OnDamage, *this, _1) );

    physical.SetOutputTransform( &position );
    physical.SetOnTouch( bind(&Player::OnTouch, *this, _1) );
    pw.AddShape( physical );

    visual.SetInputTransform( &position );
    vs.AddModel( visual );
  }
  bool OnTouch( const TouchEvent& e )
  {
    if( health.OnTouch( e ) ) return true;
    if( inventory.OnTouch( e ) ) return true;
    return false;
  }
  void OnDamage( const DamageEvent& e )
  {
    if( e.healthRemaining <= 0 ) { ... }
  }
...
};