Is this the optimal way of setting table values?

Help and Feedback Code Review
#1

My current set up is to generate upgrade values for my game as manually setting them would take ages so i used a table and a for loop to generate values based off of number factors that increase the value each time. Is this the best way to set tables values or am missing a useful function or different method?

Public.UpgradesTable = {
    PetStorage = {
        [1] = {Cost = 100, newAmount = 15, XP = 50};
    };
    EggCapacity = {
        [1] = {Cost = 100; newAmount = 10, XP = 50}
    };
    PetsEquipped = {
        [1] = {Cost = 100; newAmount = 3, XP = 50}
    };
}

for i = 2, 15, 1 do
    table.insert(Public.UpgradesTable.PetStorage, {
        Cost = Public.UpgradesTable.PetStorage[i-1].Cost * UpgradePetStorageCostFactor;
        newAmount = Public.UpgradesTable.PetStorage[i-1].newAmount * UpgradePetStorageAmountFactor;
        XP = Public.UpgradesTable.PetStorage[i-1].XP * UpgradePetStorageXPFactor;
    })

    table.insert(Public.UpgradesTable.EggCapacity, {
        Cost = Public.UpgradesTable.EggCapacity[i-1].Cost * UpgradeEggCapacityCostFactor;
        newAmount = Public.UpgradesTable.EggCapacity[i-1].newAmount * UpgradeEggCapacityAmountFactor;
        XP = Public.UpgradesTable.EggCapacity[i-1].XP * UpgradeEggCapacityXPFactor;
    })

    table.insert(Public.UpgradesTable.PetsEquipped, {
        Cost = Public.UpgradesTable.PetsEquipped[i-1].Cost * UpgradePetsEquippedCostFactor;
        newAmount = Public.UpgradesTable.PetsEquipped[i-1].newAmount * UpgradePetsEquippedAmountFactor;
        XP = Public.UpgradesTable.PetsEquipped[i-1].XP * UpgradePetsEquippedXPFactor;
    })
end
#2

Personally, I wouldn’t use a dictionary at all unless absolutely necessary. What I do instead is create a function that returns a value for the next upgrade and I only call it when I need my code to recognise a new cost for any reason, from security checks to updating Guis.

Other than that idea of changing the way you generate your values, yes, your code looks fine as it is. One thing I’d recommend is to store some of your indexes into variables so that you aren’t constantly performing table lookups and writing the same code over again.

for i = 2, 15 do
    local PetStorage = Public.UpgradesTable.PetStorage
    local PreviousPet = PetStorage[i-1]

    table.insert(PetStorage, {
        Cost = PreviousPet.Cost * UpgradePetStorageCostFactor;
        newAmount = PreviousPet.newAmount * UpgradePetStorageAmountFactor;
        XP = PreviousPet.XP * UpgradePetStorageXPFactor;
    }

    -- and so on
#3

Out of curiosity, is there a reason why you starting the loop from two and going to 15 when you’re just subtracting one from the value of I to get the index? It seems like a smarter option would be to go from one to 14 as it would have the same effect.

#4

Good to see you again Jack. There are few questionable practices I see in your script.

  • I don’t see why you do [1] when we already have numeric indices.

  • If your goal is to append elements you should be using table[#array] however they may have optimized table.insert.

  • You should consider creating variables to reduce that extraneous indexing, specially since its inside that for loop.

  • You should consider either putting that UpgradesTable inside Public or declare UpgradeTable on its own to reduce indexing here too.

#5

From a quick look at the code, it seems that the first indice is set as the defaults for the value. That is then used as a reference point for the next upgrade where table.insert creates the next indice based off of values from the previous one, which is why the loop begins at 2 and not 1 and goes to 15 not 14.

The effects between your suggestion and what the code is doing would not have the same effect, because then the indice of [i-1] where i = 1 would be nil and thus that provides no data to work off of when the next indice is being created and inserted into the table.

Again, why in cases like this I’d prefer writing a function that calculates the cost arbitrarily instead of using a dictionary and then storing that value somewhere for later use.

#6

Ah, you’re right! I completely looked over that part. From my very quick glance, it looked like they were just accessing the dictionary. Oops.