Ball Entity for a Entity/Component/System Soccer Game

Question

Here is a code snippet of a class that I be interested to refactor. I was in doubt on how to proceed with the process of instantiation. The original ask was here. After think about it and heard some opinions I end up refactoring and splitting the instantiation in several constructors, one public and all others private. what do you think of this approach?

I take this opportunity to ask you to evaluate other aspects of the code.

public class BallEntity extends UserEntity {
 public static final float SCALE_FACTOR = 0.78f;
 public static final String BALL = "ball";
 private Body ballBody;
 private Camera camera;
 private Sprite ball;
 private Fixture ballFixture;
 public BallEntity(TextureAtlas atlas, RubeSceneHelper rubeSceneHelper, Camera camera) {
 this(ScaledSprite.createUsingHeight(new Sprite(atlas.findRegion(BALL)), SCALE_FACTOR), rubeSceneHelper);
 this.camera = camera;
 }
 private BallEntity(ScaledSprite scaledSprite, RubeSceneHelper rubeSceneHelper) {
 this(scaledSprite.getSprite(), rubeSceneHelper.getBody(BALL), rubeSceneHelper);
 }
 private BallEntity(Sprite sprite, Body ballBody, RubeSceneHelper rubeSceneHelper) {
 this(rubeSceneHelper.getFixture(ballBody, BALL));
 this.ball = sprite;
 this.ballBody = ballBody;
 }
 private BallEntity(Fixture ballFixture) {
 this.ballFixture = ballFixture;
 }
 @Override
 public Array<Component> getComponents() {
 Array<Component> components = new Array<Component>();
 components.add(PositionComponent.newInstance());
 components.add(CameraFollowerComponent.newInstance(camera));
 components.add(SpriteComponent.newInstance(ball));
 components.add(BodyComponent.newInstance(ballBody));
 components.add(BallContextComponent.newInstance());
 return components;
 }
 @Override
 public void init(Entity entity) {
 BodyComponent bodyComponent = entity.getComponent(BodyComponent.class);
 bodyComponent.setPosition(Vector2.Zero);
 ballFixture.setUserData(new FixtureUserData(FixtureType.BALL, entity));
 }
}
public abstract class UserEntity {
 private final Entity entity;
 private boolean wasBuilt = false;
 private Array<Class<? extends Component>> componentClasses;
 protected UserEntity() {
 this.entity = new Entity();
 }
 public final Entity getEntity() {
 if (!wasBuilt) {
 final Array<Component> components = getComponents();
 componentClasses = new Array<Class<? extends Component>>();
 for (Component c : components) {
 entity.add(c);
 componentClasses.add(c.getClass());
 }
 init(entity);
 }
 return entity;
 }
 protected abstract Array<Component> getComponents();
 public Class<? extends Component>[] getComponentClasses() {
 if (!wasBuilt) {
 throw new IllegalArgumentException("It needs to build entity before use it.");
 }
 return componentClasses.toArray();
 }
 /* Override when need to init some components */
 public void init(Entity entity) {
 }
 public void addToEngine (Engine engine){
 engine.addEntity(entity);
 }
}

Answer 1

Looking at the class from the outside, this is how it gets created:

public BallEntity(TextureAtlas atlas, RubeSceneHelper rubeSceneHelper, Camera camera)

How the class actually gets created requires jumping between a sequence of private constructors... that isn't needed at all.

public BallEntity(TextureAtlas atlas, RubeSceneHelper rubeSceneHelper, Camera camera) {
 this.camera = camera;
 this.ballFixture = rubeSceneHelper.getFixture(ballBody, "ball");
 Sprite sprite = new Sprite(atlas.findRegion(BALL))
 this.ball = ScaledSprite.createUsingHeight(sprite, SCALE_FACTOR).getSprite();
 this.ballBody = rubeSceneHelper.getBody("ball");
}

Here I've introduced an intermediate Sprite object variable, only because all those chained calls wouldn't look right.

Isn't it easier to follow? :)

---

This seems pretty verbose:

public Array<Component> getComponents() {
 Array<Component> components = new Array<Component>();
 components.add(PositionComponent.newInstance());
 components.add(CameraFollowerComponent.newInstance(camera));
 components.add(SpriteComponent.newInstance(ball));
 components.add(BodyComponent.newInstance(ballBody));
 components.add(BallContextComponent.newInstance());
 return components;
}

I'm more into C#, but I'm pretty sure (make it 51%) you could do this instead (assuming Array<T> is intended to work like a T[]):

public Component[] getComponents() {
 return new Component[] {
 PositionComponent.newInstance(),
 CameraFollowerComponent.newInstance(camera),
 SpriteComponent.newInstance(ball),
 BodyComponent.newInstance(ballBody),
 BallContextComponent.newInstance()
 };
}

• maaartinus: With array initializers, you can even put a comma after the last element, which makes rearranging them much easier.
• alexpfx: You are right, I like this kind o syntax. I don't remember why I didn't do like getComponentClasses(). But I will change.

Answer 2

public BallEntity(TextureAtlas atlas, RubeSceneHelper rubeSceneHelper, Camera camera) {
 this(ScaledSprite.createUsingHeight(new Sprite(atlas.findRegion(BALL)), SCALE_FACTOR), rubeSceneHelper);

I'm not a fan of creating new objects in constructors. The Sprite is a dependency and dependencies should be created outside of the class and injected in through the constructor. However, it looks like the creation of the dependencies is pretty complicated, so I do see why you chose to consolidate it in one place. The constructor just isn't the place to do it. Consider making yourself a factory class or using an inversion of control container.