5
\$\begingroup\$

I am using "AndEngine" and I would like to know if I am following the best practice when I want to refresh the entity.

Let say, I have 2 sprites and I want change them into 2 another sprites.

I usually do it like this :

class MainClass {
    private Sprite sprite1;
    private Sprite sprite2;

    private void someMethod(){
        sprite1 = new Sprite(0,0,someRegion1,getVertexBufferObjectManager());
        sprite2 = new Sprite(10,10,someRegion2,getVertexBufferObjectManager());

        attachChild(sprite1);
        attachChild(sprite2);
    }

    private void refreshSprites(Sprite pSprite1, Sprite pSprite2){
        Sprite temp1 = sprite1;
        Sprite temp2 = sprite2;
        sprite1 = sprite2 = null;
        runOnUpdateThread(new Runnable() {
                            @Override
                            public void run() {
                                temp1.detachSelf();
                                temp2.detachSelf();
                            }
                        });
        sprite1 = pSprite1;
        sprite2 = pSprite2;

        attachChild(sprite1);
        attachChild(sprite2);
    }
}
\$\endgroup\$
1
\$\begingroup\$

I am not familiar with AndEngine, but here are some things that I saw:

Inside your refreshSprites method you call detachSelf and attachChild in unsynchronized way. So either make sure this method is called from updateThread or make all those operation in your Runnable class. Here is an example of the second choice.

private void refreshSprites(Sprite pSprite1, Sprite pSprite2){
        final Sprite oldSprite1 = sprite1;
        final Sprite oldSprite2 = sprite2;
        final Sprite newSprite1 = pSprite1;
        final Sprite newSprite2 = pSprite2;

        sprite1 = sprite2 = null;

        runOnUpdateThread(new Runnable() {
                            @Override
                            public void run() {
                                oldSprite1.detachSelf();
                                oldSprite2.detachSelf();
                                attachChild(newSprite1);
                                attachChild(newSprite2);
                            }
                        });

    }

Pay attention to the final that I am using, otherwise those values will not be available in the oldSprite1 inner class.

And some small things

  • sprite1 = sprite2 = null; you should place in it's own method, call it reset or some thing similar. It will improve your method Single responsibility principle
  • Pay attention to my choice of variables names: oldSprite, I think it explain the meaning of the variable better than tmp.
\$\endgroup\$
  • \$\begingroup\$ oo I totaly forget about sprite1.setVisible(false) I will edit my question :). Thanks for tip about variable naming but this is only for example. But in other hand this is code review so code should be elegant in any case :). Got to edit it. \$\endgroup\$ – Błażej Jun 26 '14 at 16:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.