Design: Having references in the subclass of a field in the superclass

Question

First of all, I have no idea if the title even reflects my question. I apologize and kindly request to change it if you have a better idea.

Now, the problem I have is mainly a design problem. I am not sure if what I want is a good practice but it's d*mn convenient.

I have an Interface called IMovement.

Two classes implement IMovement: VelocityMovement, SplineMovement.

I also have a class called Sprite. A Sprite has an IMovement.

Finally, I have two sublcasses of Sprite: Player and Enemy.

I KNOW that Player will ALWAYS have VelocityMovement and
I KNOW that Enemy will ALWAYS have SplineMovement.

Is it wrong (either by design or practical) to have, aside from the superclasses reference to IMovement, an attribute to the VelocityMovement or SplineMovement from Player or Enemy respectively?

In other words:

Interface IMovement {
  public Vector2 getPosition(int time);
}

class VeloctiyMovement implements IMovement { 
   public Vector2 velocity;

   public Vector2 getPosition(int time) { ... }
}

class SplineMovement implements IMovement { 
   public Spline spline;

   public Vector2 getPosition(int time) { ... }
}

class Sprite {
  IMovement movement;
}

class Player extends Sprite {
  public VelocityMovement velocityMovement;

  public Player() {
    velocityMovement = new VelocityMovement();
    movement = velocityMovement;
  }
}

class Enemy extends Sprite {
  public SplineMovement splineMovement;

  public Enemy() {
    splineMovement = new SplineMovement();
    movement = splineMovement();
  }
}

The reason I want to have the reference in the subclass is convenience. Half the times I need the velocity field from Player, and half the other times I don't care. This happens in a game where the method gets called approximately 100 times per second.

The reason I am pointing out the 100 times is because I also am concerned if casting is expensive.

Example, instead of:

// When I need the velocity
Vector2 velocity = ((VelocityMovement)player.movement).velocity;
...

// When I don't care
Vector2 position = player.movement.GetPosition(time);
...

I can now do this:

Vector2 velocity = player.VelocityMovement.velocity;
...

Vector2 position = player.movement.GetPosition(time);
...

So my question is: Should Player have a VelocityMovement field and an Enemy have a SplineMovement? Or is this a bad design?

I would like your thoughts.

UPDATE

I now remembered why I chose to have a field IMovement instead of having Sprite implement IMovement (as JacobM correctly suggests).

It is because in some cases, Enemy could have SplineMovement and in others VelocityMovement.

Although I, incorrectly, said "I KNOW that Enemy will ALWAYS have SplineMovement", I should have stated that, I know an Enemy has VelocityMovement in places where I gather all enemies with VelocityMovements. I hope you understand me (otherwise I feel like an idiot).

For example:

List<Enemy> enemies = GetAllVelocityMovementEnemies();

I do this because somewhere in my game logic, I want to affect only the group of enemies that have VelocityMovements.

So, although JacobM's answer is perfectly valid as a design, this is not for my case. Otherwise I would have to subclass Enemy with EnemyVelocityMovement and EnemySplineMovement and so on and so forth.

So, sorry for the misunderstanding, I have decided that Brian Agnew's answer is the best one for my design.

Oh, and yes, waring about casting is indeed premature optimization. ;)

Answer 1

Don't worry about optimisation for the moment. Premature optimisation will cause you grief that is quite likely not warranted.

So, to answer your question, don't duplicate fields across the hierarchy - it's a maintenance headache and your perceived time-saving will be negligible. If you follow the DRY principle then you'll save yourself a lot of headaches.

Ask your classes to move (say) via something like:

player.move()

otherwise (as you're discovering), you have to find their IMovement, and then cast it, and then ask that for the velocity, and that really breaks encapsulation. Your Player/Enemy classes will likely have different move() implementations, depending on their particulars.

You may not want a move() method, but what I'm trying to demonstrate is that the intelligence is contained in the class, not in the calling code. So you shouldn't have to expose your IMovement objects at all.

Finally, I would only optimise after getting it working, and measuring it to determine that you have a problem!

Answer 2

I don't have time to try to write code, but perhaps, if the language you are using supports generic types, you could do something like

class Sprite<T extends IMovement> {
   T movement;
}

class Player extends Sprite<VelocityMovement> {
}

class Enemy extends Sprite<SplineMovement>{
}

This way, when you ask for the player's movement object, you know already that it's a VelocityMovement (but of course you can also refer to it as an IMovement).