Programming Dilemma: Repetition vs Performance

I am stuck in between a decision.

I need to choose between code repetition or program performance.

Currently, I have 100 asteroids on screen and I am planning on having 5 different types of enemy ships (all capable of colliding into asteroids).

My question is should I write the collision code in the asteroid.lua file and place the enterFrame and collision listener on them (Meaning 100 Collision Listeners) or should I place it for each individual ship. (Meaning a block code would be repeated around 6 times throughout the project, but only a max of about 15 to 20 Collision Listeners).

Also, this is the code in question:

if event and event.other and event.phase == "began" then if event.other.id == "bullet" or event.other.type == "ship" then event.other.health = event.other.health - self.damage self.health = self.health - event.other.damage if self.health \<= 0 then explosion.new(self.parent, self.x, self.y, 1, 1, math.random(-10, 10), math.random(-10, 10)) display.remove(self) end end end

So, what should I do?

I am expecting to potentially add more asteroids than 100, maybe 250. I am thinking I’d rather repeat the code, than have 250 listeners all at once. But at the same time, they will only be triggered on collision…

What do you guys think?

PS: I am aware I would have to make modifications to the code if I go for the repetition option.

Why not put your code in a module, define it once, then require the module where you need it and use the functions/methods that way.

Defining the functions once is better is so many ways (smaller memory usage, easier to maintain, less error prone, …)

I think this (not a module, just showing re-use) is better,

local function onEnterFrame( self ) -- ... end local function onCollision( self, event ) -- ... end for i = 1, 1000 do local obj = -- ... obj.collision = onCollision obj.enterFrame = onEnterFrame obj:addEventListener("collision") listen("enterFrame", obj ) end

than this,

for i = 1, 1000 do local obj = -- ... function obj.onEnterFrame( self ) -- ... end function obj.onCollision( self, event ) -- ... end obj:addEventListener("collision") listen("enterFrame", obj ) end

Tip: You should localize any fields you use a lot.  This saves typing and reduces errors and typos:

local other = event.other local phase = event.phase local mRand = math.random if( phase == "began" ) then -- You can't have a phase 'began' without an other if( other.id == "bullet" or other.type == "ship" ) then other.health = other.health - self.damage self.health = self.health - other.damage if( self.health \<= 0 ) then explosion.new(self.parent, self.x, self.y, 1, 1, mRand(-10, 10), mRand(-10, 10)) display.remove(self) end end end

Also, I hope you are not saying you intend to use the same listener code for two different object types.

That is not a good idea.  It is better to have all listeners be specific to an object type.

That is …

  • one collision listener definition for bullets.
  • one for player
  • one for asteroids

Otherwise your code complexity will grow without bound and eventually be full of bugs.  Also, your logic will become twisted and have weird corner cases.

Finally, not all objects need listeners.  Handle the game logic in as few listeners as possible (still specific to a single object type/category).

PS - The code you posted and that I refined is fine.  I assume that is the asteroid listener code.

Thank you. The asteroid listener collision function was outside the .new() function, I was just making sure which would be better. 

Why not put your code in a module, define it once, then require the module where you need it and use the functions/methods that way.

Defining the functions once is better is so many ways (smaller memory usage, easier to maintain, less error prone, …)

I think this (not a module, just showing re-use) is better,

local function onEnterFrame( self ) -- ... end local function onCollision( self, event ) -- ... end for i = 1, 1000 do local obj = -- ... obj.collision = onCollision obj.enterFrame = onEnterFrame obj:addEventListener("collision") listen("enterFrame", obj ) end

than this,

for i = 1, 1000 do local obj = -- ... function obj.onEnterFrame( self ) -- ... end function obj.onCollision( self, event ) -- ... end obj:addEventListener("collision") listen("enterFrame", obj ) end

Tip: You should localize any fields you use a lot.  This saves typing and reduces errors and typos:

local other = event.other local phase = event.phase local mRand = math.random if( phase == "began" ) then -- You can't have a phase 'began' without an other if( other.id == "bullet" or other.type == "ship" ) then other.health = other.health - self.damage self.health = self.health - other.damage if( self.health \<= 0 ) then explosion.new(self.parent, self.x, self.y, 1, 1, mRand(-10, 10), mRand(-10, 10)) display.remove(self) end end end

Also, I hope you are not saying you intend to use the same listener code for two different object types.

That is not a good idea.  It is better to have all listeners be specific to an object type.

That is …

  • one collision listener definition for bullets.
  • one for player
  • one for asteroids

Otherwise your code complexity will grow without bound and eventually be full of bugs.  Also, your logic will become twisted and have weird corner cases.

Finally, not all objects need listeners.  Handle the game logic in as few listeners as possible (still specific to a single object type/category).

PS - The code you posted and that I refined is fine.  I assume that is the asteroid listener code.

Thank you. The asteroid listener collision function was outside the .new() function, I was just making sure which would be better.