How do i make this more readable/"dry"

Below is embedded the code I am querying about. So overall the code design in “upgrade.lua” is designed so I am able to easily create new images in “upgrade.lua” without making any more functions. Its also done this way so I don’t have to create another database. 

I have likely made a ‘logic’ error related to the SQLite database and a change would make everything much easier. However, I would greatly appreciate any feedback on how to improve this code.

utils.lua (settings) - 

--tables utils.settings = {--id, name, value, cost, + table num, image, x offset, y offset { 1, "balance" , 0, 0 , 0, "", }, { 2, "gamercoin" , 0, 0 , 0, "", }, { 3, "clickervalue" , 1, 0 , 0, "", }, { 4, "autoclicker" , 1, 0 , 0, "", }, { 5, "monitor\_level", 4, 0 , 0, "", }, { 6, "buymouse" , 1, -20, 1, "mouse1", 100, 75 }, --make sure the value increases by two each time }

utils.lua (database) - 

--updating stuff for row in db:nrows( "SELECT \* FROM database" ) do if ( row.variable\_name == utils.settings[row.id][2] ) then utils.settings[row.id][3] = row.value utils.settings[row.id][4] = row.upgrade end end --the ONLY PUBLIC FUNCTION TO GO here function utils.update\_database( id, variable\_name, value, upgrade ) local temp = [[DELETE FROM database WHERE id =]] .. id .. [[;]] db:exec( temp ) local temp = [[INSERT INTO database VALUES (]] .. id .. [[, "]] .. variable\_name .. [[",]] .. value .. [[,]] .. upgrade .. [[);]] db:exec ( temp ) end

upgrade.lua - 

local objects = {} local function increase\_ugrades( event ) if ( event.phase == "ended" ) then for i = 1, #utils.settings do if ( event.target.string == utils.settings[i][2] ) then if ( utils.settings[1][3] + utils.settings[i][4] \>= 0 ) then if ( objects[utils.settings[i][5]].alpha == 1 ) then utils.update\_balance( utils.settings[i][4] ) if ( utils.settings[i][6] ~= "" and utils.settings[i][3] \< 2 ) then utils.settings[i][3] = 2 elseif ( utils.settings[i][6] == "" ) then utils.settings[i][3] = utils.settings[i][3] + 1 end if ( utils.settings[i][6] == "" ) then utils.settings[i][4] = utils.settings[i][4] \* 2 end if ( utils.settings[i][6] ~= "" ) then objects[utils.settings[i][5]].alpha = 0.5 objects[utils.settings[i][5] + 1].alpha = 0.5 end objects[utils.settings[i][5] + 1].text = utils.settings[i][2] .. ": " .. ( -1 \* utils.settings[i][4] ) utils.update\_database( utils.settings[i][1], utils.settings[i][2], utils.settings[i][3], utils.settings[i][4] ) end end end end end end for i = 1, #utils.settings do if ( utils.settings[i][4] ~= 0 ) then objects[utils.settings[i][5] + 1] = display.newText( utils.settings[i][2] .. ": " .. ( -1 \* utils.settings[i][4] ), center\_x, center\_y + 50, font, 17) sceneGroup:insert( objects[utils.settings[i][5] + 1] ) objects[utils.settings[i][5]] = display.newImage( utils.settings[i][2] .. ".png", center\_x, center\_y ) sceneGroup:insert( objects[utils.settings[i][5]] ) objects[utils.settings[i][5]]:scale( 0.4, 0.4 ) if ( utils.settings[i][3] == 2 ) then objects[utils.settings[i][5]].alpha = 0.5 objects[utils.settings[i][5] + 1].alpha = 0.5 end objects[utils.settings[i][5]].string = utils.settings[i][2] objects[utils.settings[i][5]]:addEventListener( "touch", increase\_ugrades ) end end

I haven’t got time right now to pick through that, however the first improvement you can make is to wrap everything in this:

[lua]

db:exec(“BEGIN TRANSACTION”)

db:exec(“COMMIT”)

[/lua]

This will defer actually writing to the disk until all queries have been setup, which is quicker and kinder on the disk.

Given your structure I guess you’ll need another couple of public functions within utilities that allow you to begin and commit a transaction.

Sorry I am rather new to SQLite… To my understanding, I would want to ‘BEGIN TRAN’ before adding/changing a value in the database, and ‘COMMIT’ afterward.

I’m unsure how to incorporate this into “utils.lua” (sorry).

I read this: https://www.sqlite.org/lang_transaction.html 

But I’m still confused. More help would be greatly appreciated.

You would want to call BEGIN TRANSACTION before you start all those loops off, and COMMIT after you’ve finished. This wraps all the sql statements into one transaction, rather than one for each insert/update.

As utils.lua is the file with the connection to the database, you would need two functions in there that can be called from upgrade.lua, before and after you do the work.

updated (upgrades.lua) - 

 local function increase\_upgrades( event ) if ( event.phase == "ended" ) then for i = 1, #utils.settings do if ( event.target.string == utils.settings[i][2] ) then if ( utils.settings[1][3] + utils.settings[i][4] \>= 0 ) then --if the users balance + objects cost \>= 0 then if ( objects[utils.settings[i][5]].alpha == 1 ) then --if the object has a alpha value of 1 then utils.update\_balance( utils.settings[i][4] ) --update the balance by deducting the cost of the object if ( utils.settings[i][6] ~= "" and utils.settings[i][3] \< 2 ) then --if the object can draw an image then utils.settings[i][3] = 2 --these objects only get brought once objects[utils.settings[i][5]].alpha = 0.5 --set alpha to 0.5 objects[utils.settings[i][5] + 1].alpha = 0.5 --set alpha to 0.5 (they now cannot be brought again) elseif ( utils.settings[i][6] == "" ) then --if the object cannot draw an image then utils.settings[i][3] = utils.settings[i][3] + 1 --increase the value of the object by 1 utils.settings[i][4] = utils.settings[i][4] \* 2 --doubling the cost of the object utils.settings[3][3] = utils.settings[3][3] + 1 --increasing the clickervalue by 1 end objects[utils.settings[i][5] + 1].text = utils.settings[i][2] .. ": " .. ( -1 \* utils.settings[i][4] ) --updating the text associated with the brought object utils.update\_database( utils.settings[i][1], utils.settings[i][2], utils.settings[i][3], utils.settings[i][4] ) --updating the objects value in the database end end end end end end

I’m not sure if I am misunderstanding you or not but from my understanding you *might* be thinking that ‘local function increase_upgrades( event )’ contains a loop that loops more than once (at a given time). The loops conditional statements limit the loop to only completing one cycle/loop. So ‘utils.update_database’ is only called once per touch event.

Does this change what you’re saying or did I misunderstand your comment  :blink: 

Ah I see, you’re all good then :slight_smile:

but for future, if I were to add more than one ‘utils.update_database’ per touch event should I wrap them in ‘BEGIN TRANSACTION’ and ‘COMMIT’?

I love posts that assume the reader will know what 

utils.settings[1][3] + utils.settings[i][4]

will actually mean!

i know it annoying but i did post the table in the OP

what is the problem you have?

there is no problem i am just querying how i can improve the quality of my code/readibility

If your code is not broken? then don’t worry about it.

but always wrap DB code in BEGIN/END transaction code

I haven’t got time right now to pick through that, however the first improvement you can make is to wrap everything in this:

[lua]

db:exec(“BEGIN TRANSACTION”)

db:exec(“COMMIT”)

[/lua]

This will defer actually writing to the disk until all queries have been setup, which is quicker and kinder on the disk.

Given your structure I guess you’ll need another couple of public functions within utilities that allow you to begin and commit a transaction.

Sorry I am rather new to SQLite… To my understanding, I would want to ‘BEGIN TRAN’ before adding/changing a value in the database, and ‘COMMIT’ afterward.

I’m unsure how to incorporate this into “utils.lua” (sorry).

I read this: https://www.sqlite.org/lang_transaction.html 

But I’m still confused. More help would be greatly appreciated.

You would want to call BEGIN TRANSACTION before you start all those loops off, and COMMIT after you’ve finished. This wraps all the sql statements into one transaction, rather than one for each insert/update.

As utils.lua is the file with the connection to the database, you would need two functions in there that can be called from upgrade.lua, before and after you do the work.

updated (upgrades.lua) - 

 local function increase\_upgrades( event ) if ( event.phase == "ended" ) then for i = 1, #utils.settings do if ( event.target.string == utils.settings[i][2] ) then if ( utils.settings[1][3] + utils.settings[i][4] \>= 0 ) then --if the users balance + objects cost \>= 0 then if ( objects[utils.settings[i][5]].alpha == 1 ) then --if the object has a alpha value of 1 then utils.update\_balance( utils.settings[i][4] ) --update the balance by deducting the cost of the object if ( utils.settings[i][6] ~= "" and utils.settings[i][3] \< 2 ) then --if the object can draw an image then utils.settings[i][3] = 2 --these objects only get brought once objects[utils.settings[i][5]].alpha = 0.5 --set alpha to 0.5 objects[utils.settings[i][5] + 1].alpha = 0.5 --set alpha to 0.5 (they now cannot be brought again) elseif ( utils.settings[i][6] == "" ) then --if the object cannot draw an image then utils.settings[i][3] = utils.settings[i][3] + 1 --increase the value of the object by 1 utils.settings[i][4] = utils.settings[i][4] \* 2 --doubling the cost of the object utils.settings[3][3] = utils.settings[3][3] + 1 --increasing the clickervalue by 1 end objects[utils.settings[i][5] + 1].text = utils.settings[i][2] .. ": " .. ( -1 \* utils.settings[i][4] ) --updating the text associated with the brought object utils.update\_database( utils.settings[i][1], utils.settings[i][2], utils.settings[i][3], utils.settings[i][4] ) --updating the objects value in the database end end end end end end

I’m not sure if I am misunderstanding you or not but from my understanding you *might* be thinking that ‘local function increase_upgrades( event )’ contains a loop that loops more than once (at a given time). The loops conditional statements limit the loop to only completing one cycle/loop. So ‘utils.update_database’ is only called once per touch event.

Does this change what you’re saying or did I misunderstand your comment  :blink: 

Ah I see, you’re all good then :slight_smile:

but for future, if I were to add more than one ‘utils.update_database’ per touch event should I wrap them in ‘BEGIN TRANSACTION’ and ‘COMMIT’?

I love posts that assume the reader will know what 

utils.settings[1][3] + utils.settings[i][4]

will actually mean!