How do I simplify my code?

[sheikhameen123]

I’m a newbie, and learning stuff by practicing code. I’ve made a simple app (too simple ;experimental), and I feel like I’ve been repeating codes again and again. Can someone help me simplify the code, so that I can learn from the examples…  :)  My app works like this: When you click/touch the background, a physical object circle is created and falls due to gravity. The display has borders(rectangles to prevent the balls from falling off screen) and a few rectangles in the middle just for fun :P . After 10 seconds, the balls start to remove from display(but not sure if it is freed from memory). Balls have different colors when clicked/touched at different fractions of the width. Also help me to add a new color to the last fraction of the width. When I attempt it, the color doesn’t appear. I want to see four colors but instead I am getting three. Thank you for anyone who put in effort to help me learn. 

----------------------------------------------------------------------------------------- -- -- main.lua -- ----------------------------------------------------------------------------------------- display.setStatusBar(display.HiddenStatusBar) local physics = require("physics") physics.start() physics.setGravity(0,50) local background = display.newImageRect( "background.png", 800, 600 ) background.x = display.contentCenterX background.y = display.contentCenterY local function gameLoop(event) if ( event.x \< display.contentCenterX / 2) then local shape = display.newCircle( event.x, event.y, 5 ) physics.addBody( shape, "dynamic", {density=1, radius=6}) shape:setFillColor(0.2, 0.7, 1, 0.5) local function removeCircle() display.remove(shape) end timer.performWithDelay( 10000, removeCircle) elseif ( event.x \< display.contentCenterX ) then local shape = display.newCircle( event.x, event.y, 5 ) physics.addBody( shape, "dynamic", {density=1, radius=6}) shape:setFillColor(1, 0, 0, 0.5) local function removeCircle() display.remove(shape) end timer.performWithDelay( 10000, removeCircle) elseif ( event.x \< display.contentCenterX \* 2) then local shape = display.newCircle( event.x, event.y, 5 ) physics.addBody( shape, "dynamic", {density=1, radius=6}) shape:setFillColor(0.349, 0.156, 0.423, 0.5) local function removeCircle() display.remove(shape) end timer.performWithDelay( 10000, removeCircle) end end background:addEventListener("touch", gameLoop) --[[timer.performWithDelay(10, gameLoop, -1) --]] local rectangle1 = display.newRect( display.contentCenterX / 4, display.contentCenterY / 4, display.contentWidth, 10 ) rectangle1.rotation = 20 local rectangle2 = display.newRect( display.contentWidth / 1.3, display.contentCenterY/ 1.3, display.contentWidth, 10 ) rectangle2.rotation = -20 local rectangle3 = display.newRect( display.contentCenterX / 4, display.contentCenterY / 0.7, display.contentWidth, 10 ) rectangle3.rotation = 20 local rectangle4 = display.newRect( display.contentWidth / 1.3, display.contentCenterY / 0.5, display.contentWidth, 10 ) rectangle4.rotation = -20 local rectangle5 = display.newRect( display.contentCenterX, display.contentHeight + 40, display.contentWidth, 10 ) rectangle5.rotation = 0 local rectangle6 = display.newRect( display.contentWidth - 5, display.contentCenterY, display.contentHeight + 70, 10 ) rectangle6.rotation = 90 local rectangle7 = display.newRect( 5, display.contentCenterY, display.contentHeight + 70, 10 ) rectangle7.rotation = 90 local rectangle8 = display.newRect( display.contentCenterX, -40, display.contentWidth, 10 ) rectangle8.rotation = 0 physics.addBody( rectangle1, "static") physics.addBody( rectangle2, "static") physics.addBody( rectangle3, "static") physics.addBody( rectangle4, "static") physics.addBody( rectangle5, "static") physics.addBody( rectangle6, "static") physics.addBody( rectangle7, "static")

[cyberparkstudios]

I don’t have a lot of time to look at it real closely but I can say there are a hundred ways to do everything in coding, and what I suggest is only one way.  Ideally, it is great to ‘reduce’ or eliminate ‘repetitive’ coding… but some times it can’t be helped and sometimes, it is as much work to avoid it as to do it.

I have little experience with physics so I cannot comment much on that stuff.

First, normal practice would suggest declare most of the variables, such as your rectangles(1 thru 8) near the top of your module, above your game loop. (see the sample code I have below)

In your case having 8 rectangles in such a small application you are ‘almost’ okay to just do it the way you have it.  For example if I had an app and had 3 maybe 4 rectangles I would likely repeat code and declare and define each of the rectangles ‘a little bit’ as you did.  Especially since you have a variety of positioning formulas, it would be easier for me to do it similar to that.  But, if I have more then say 4 similar objects I am going to use a table. (if it is a more detailed object then say a standard rectangle - I create a pseudo-class and create an object of each of them -which reduces a lot of repetitive code)…  But for now, in this example tables work to help reduce repeating code.

I would do something like this below your background declaration and above you game loop stuff.

local cw = display.contentWidth local ch = display.contentHeight local ccx = display.contentCenterX local ccy = display.contentCenterY local posX = { ccx, cw / 1.3, ccx / 4, cw / 1.3, cw / 1.4, cw - 5, 5, ccx, } local posY = { --same thing for the y positions in this table } local rotations = {20, -20, 20, -20, 0, 90, 90, 0} local rects = {} for i = 1, 8 do table.insert(rects, display.newRect( 0, 0, display.contentWidth, 10 )) rects[i].rotation = rotations[i] rects[i].x = posX[i] rects[i].y = posY[i] physics.addBody( rects[i], "static") end

you could also avoid the posX and posY table and after doing the loop creating the rectangles, then individually set the x and y for each rect like this :

rects[1].x = whatever

rects[1].y = whatever

rects[2].x …

hope this helps.

[cyberparkstudios]

also, I think there is somethings that can be probably be  improved in the game loop… I just did not have timer to look at that.  If I have some time later today, I may take another look and see what might be helpful.

[XeduR]

Also, remember that making your code simpler is not just about making it shorter.

@cyberparkstudios gave you a plenty of useful tips already. As he said, using loops (or functions) to create objects instead of manually repeating the same code and just changing a few values is the way to go. However, you should also focus on code readability. For instance,

 

-- having these x coordinates in a table is great local posX = { ccx, cw / 1.3, ccx / 4, cw / 1.3, cw / 1.4, cw - 5, 5, ccx, } -- however, if you will have other objects in your game that would also be created -- using a similar loop, then perhaps you should consider using a more descriptive -- name for the table so that identifying the tables would be easier, e.g rectsPosX. -- then, to further improve the readability, you could split the table into entries: local rectsPosX = {} rectsPosX[1] = ccx rectsPosX[2] = cw / 1.3 rectsPosX[3] = ccx / 4 rectsPosX[4] = cw / 1.3 rectsPosX[5] = cw / 1.4 rectsPosX[6] = cw - 5 rectsPosX[7] = 5 rectsPosX[8] = ccx -- this method is a bit longer, but if you think that you may want to or need to -- adjust these values at some point, then by having split the table into entries -- like this, you'll be able to idenfity each entry immediately -- now, as for your game loop, you pretty much have the same issue -- as before, i.e. the only difference between what happens within -- each condition is what colour you set, so remove all repetition local function gameLoop(event) local shape = display.newCircle( event.x, event.y, 5 ) physics.addBody( shape, "dynamic", {density=1, radius=6}) if ( event.x \< display.contentCenterX / 2) then shape:setFillColor(0.2, 0.7, 1, 0.5) elseif ( event.x \< display.contentCenterX ) then shape:setFillColor(1, 0, 0, 0.5) elseif ( event.x \< display.contentCenterX \* 2) then shape:setFillColor(0.349, 0.156, 0.423, 0.5) end local function removeCircle() display.remove(shape) end timer.performWithDelay( 10000, removeCircle) end

The entire question about simplifying one’s code ultimately comes down to personal preference, but there are some general good practices for coding that should be followed. Here’s a few good articles on the topic: https://code.tutsplus.com/tutorials/top-15-best-practices-for-writing-super-readable-code–net-8118 and https://hackernoon.com/few-simple-rules-for-good-coding-my-15-years-experience-96cb29d4acd9

Oh, and the reason why you are having problems with having three colours in your gameLoop instead of four is because: 1) you only have three conditions within the function instead of four, and 2) your math and values are incorrect.

display.contentCenterX returns the value for content area’s x centre and this depends on the values in your config.lua file, so this isn’t a dynamic value and it won’t change between different devices. I’d recommend solving the intervals by using display.screenOriginX and display.actualContentWidth. display.screenOriginX is the leftmost part of the display on any device and display.actualContentWidth is the actual width. So, the four intervals would be:

1st quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.25

2nd quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.5 (this is the same as display.contentCenterX)

3rd quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.75
4th quarter: all other values (else)

2nd quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.5 (well, also the same as display.contentCenterX)

[sheikhameen123]

Thank You so much everyone who put in effort to help me. I’ve learned a lot from you guys. This is my first attempt to make an app all by myself. I guess this is a good start, maybe. Thank you once again!

[cyberparkstudios]

I don’t have a lot of time to look at it real closely but I can say there are a hundred ways to do everything in coding, and what I suggest is only one way.  Ideally, it is great to ‘reduce’ or eliminate ‘repetitive’ coding… but some times it can’t be helped and sometimes, it is as much work to avoid it as to do it.

I have little experience with physics so I cannot comment much on that stuff.

First, normal practice would suggest declare most of the variables, such as your rectangles(1 thru 8) near the top of your module, above your game loop. (see the sample code I have below)

In your case having 8 rectangles in such a small application you are ‘almost’ okay to just do it the way you have it.  For example if I had an app and had 3 maybe 4 rectangles I would likely repeat code and declare and define each of the rectangles ‘a little bit’ as you did.  Especially since you have a variety of positioning formulas, it would be easier for me to do it similar to that.  But, if I have more then say 4 similar objects I am going to use a table. (if it is a more detailed object then say a standard rectangle - I create a pseudo-class and create an object of each of them -which reduces a lot of repetitive code)…  But for now, in this example tables work to help reduce repeating code.

I would do something like this below your background declaration and above you game loop stuff.

local cw = display.contentWidth local ch = display.contentHeight local ccx = display.contentCenterX local ccy = display.contentCenterY local posX = { ccx, cw / 1.3, ccx / 4, cw / 1.3, cw / 1.4, cw - 5, 5, ccx, } local posY = { --same thing for the y positions in this table } local rotations = {20, -20, 20, -20, 0, 90, 90, 0} local rects = {} for i = 1, 8 do table.insert(rects, display.newRect( 0, 0, display.contentWidth, 10 )) rects[i].rotation = rotations[i] rects[i].x = posX[i] rects[i].y = posY[i] physics.addBody( rects[i], "static") end

you could also avoid the posX and posY table and after doing the loop creating the rectangles, then individually set the x and y for each rect like this :

rects[1].x = whatever

rects[1].y = whatever

rects[2].x …

hope this helps.

[cyberparkstudios]

also, I think there is somethings that can be probably be  improved in the game loop… I just did not have timer to look at that.  If I have some time later today, I may take another look and see what might be helpful.

[XeduR]

Also, remember that making your code simpler is not just about making it shorter.

@cyberparkstudios gave you a plenty of useful tips already. As he said, using loops (or functions) to create objects instead of manually repeating the same code and just changing a few values is the way to go. However, you should also focus on code readability. For instance,

 

-- having these x coordinates in a table is great local posX = { ccx, cw / 1.3, ccx / 4, cw / 1.3, cw / 1.4, cw - 5, 5, ccx, } -- however, if you will have other objects in your game that would also be created -- using a similar loop, then perhaps you should consider using a more descriptive -- name for the table so that identifying the tables would be easier, e.g rectsPosX. -- then, to further improve the readability, you could split the table into entries: local rectsPosX = {} rectsPosX[1] = ccx rectsPosX[2] = cw / 1.3 rectsPosX[3] = ccx / 4 rectsPosX[4] = cw / 1.3 rectsPosX[5] = cw / 1.4 rectsPosX[6] = cw - 5 rectsPosX[7] = 5 rectsPosX[8] = ccx -- this method is a bit longer, but if you think that you may want to or need to -- adjust these values at some point, then by having split the table into entries -- like this, you'll be able to idenfity each entry immediately -- now, as for your game loop, you pretty much have the same issue -- as before, i.e. the only difference between what happens within -- each condition is what colour you set, so remove all repetition local function gameLoop(event) local shape = display.newCircle( event.x, event.y, 5 ) physics.addBody( shape, "dynamic", {density=1, radius=6}) if ( event.x \< display.contentCenterX / 2) then shape:setFillColor(0.2, 0.7, 1, 0.5) elseif ( event.x \< display.contentCenterX ) then shape:setFillColor(1, 0, 0, 0.5) elseif ( event.x \< display.contentCenterX \* 2) then shape:setFillColor(0.349, 0.156, 0.423, 0.5) end local function removeCircle() display.remove(shape) end timer.performWithDelay( 10000, removeCircle) end

The entire question about simplifying one’s code ultimately comes down to personal preference, but there are some general good practices for coding that should be followed. Here’s a few good articles on the topic: https://code.tutsplus.com/tutorials/top-15-best-practices-for-writing-super-readable-code–net-8118 and https://hackernoon.com/few-simple-rules-for-good-coding-my-15-years-experience-96cb29d4acd9

Oh, and the reason why you are having problems with having three colours in your gameLoop instead of four is because: 1) you only have three conditions within the function instead of four, and 2) your math and values are incorrect.

display.contentCenterX returns the value for content area’s x centre and this depends on the values in your config.lua file, so this isn’t a dynamic value and it won’t change between different devices. I’d recommend solving the intervals by using display.screenOriginX and display.actualContentWidth. display.screenOriginX is the leftmost part of the display on any device and display.actualContentWidth is the actual width. So, the four intervals would be:

1st quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.25

2nd quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.5 (this is the same as display.contentCenterX)

3rd quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.75
4th quarter: all other values (else)

2nd quarter: event.x <= display.screenOriginX+display.actualContentWidth*0.5 (well, also the same as display.contentCenterX)

[sheikhameen123]

Thank You so much everyone who put in effort to help me. I’ve learned a lot from you guys. This is my first attempt to make an app all by myself. I guess this is a good start, maybe. Thank you once again!