Tuesday, August 11, 2009

Getting rid of duplicate lines of code



I just finished one of MJW's sh'm'up tutorials and by the end I had multiple lines that were duplicates of each other. For example:
  1. {

  2. removeChild( bullet );

  3. bullets.splice( j, 1 );

  4. }


While this used to be okay - I just wanted working code! - Now, after these tutorials I'm spoiled. Some would say a better (though very beginner!) programmer. I feel like I need to compact it and clean it up as much as possible.
So I created a public function called removeClip (first it was removeEnemy, then I figured I could use it for the bullets too, and maybe other things later!) and passed it three variables:
  • instance of the clip I wanted to remove.
  • the position it was in the array
  • and, the array name.

    Tada!
    1. public function removeClip( movieClip:MovieClip, i:int, array:Array ):void

    2. {

    3.     removeChild( movieClip );

    4.     array.splice( i, 1 );

    5. }



    Then I just went back through AvoiderGame.as and replaced all those lines that removed the bullet / enemy with:
    removeClip( enemy, i, army );
    or
    removeClip( bullet, j, bullets );

    Michael, thanks for teaching us how to code.

    EDIT: MJW posted a comment on trimming down my code even more that I'd like to share! (Especially in case you don't read the comments...)

    He wrote: "I'm not sure if you know this, but all arrays have a method, .indexOf( item ), that returns the index of the item specified (or -1 if the item is not in the array).

    So you can write "array.splice( array.indexOf( movieClip ), 1 )", for example. This way you don't have to pass the index of the item through to the function each time!
    "

    I've used his suggestion, and changed my removeClip function to this:
    1. public function removeClip( movieClip:MovieClip, array:Array ):void

    2. {

    3.     removeChild( movieClip );

    4.     array.splice( array.indexOf( movieClip ), 1 );

    5. }


    I guess if you wanted, you could also make a check to make sure it was part of that array before trying to splice, like this:
    1. public function removeClip( movieClip:MovieClip, array:Array ):void

    2. {

    3.     removeChild( movieClip );

    4.     if( array.indexOf( movieClip ) != -1  )

    5.     {

    6.         array.splice( array.indexOf( movieClip ), 1 );

    7.     }

    8. }


    Anyways, it might not seem like a lot at first. You're only slimming it down by one small variable. But that's one less variable for every bullet that leaves the screen, or hits an enemy, plus every enemy that gets hit, or leaves the screen, and in a much bigger game it would really start adding up.

    A little more information about it here from Senocular
  • 2 comments:

    1. Excellent! Really great to see you cleaning up my messes, haha :P

      May I suggest a further addition to your idea? I'm not sure if you know this, but all arrays have a method, .indexOf( item ), that returns the index of the item specified (or -1 if the item is not in the array).

      So you can write "array.splice( array.indexOf( movieClip ), 1 )", for example. This way you don't have to pass the index of the item through to the function each time! And I'm sure you could think of an even cleverer way to use it... ;)

      ReplyDelete
    2. Hey, kewl! It worked! :) I'll post an edit to my article. That's a lot of extra information that doesn't have to be passed back and forth *every* time a bullet hits something, or leaves the screen.
      Danke!

      ReplyDelete