Datastore Script - Overhaul

Looks decent. A few things I would like to mention.

  1. Never globalize your functions unless the function is a key in a table. Globals are often never used since they are store in the heap and are slower to reference than the stack. It’s good practice to always localize them.

  2. In your ‘toBoolean’ function, returning nil is pointless. When a function returns nothing, it automatically returns nil. ‘nil’ means nothing after all.

  3. Yes, I understand this is premature optimization. But, you should never use the second argument of Instance.new()
    A thread on why Instance.new() is not recommended.

  4. If you are appending a new key into an array, array[#array + 1] would be a more optimal solution than table.insert. Yes, I’m unnecessarily optimizing by 0.01 again :((.

  5. When you iterate through the children of an instance, always use ipairs as your iterator factory. The :GetChildren method returns an array and therefore ipairs would be a better choice than pairs if we are talking about speed. Pairs would be better to use when iterating through dictionaries.

  6. Cool trick: In your ‘convertInstanceToData’ function, short circuit programming would be a good implementation. (This is not needed at all, I’m not discouraging if statements).

returnedData[v.Name] = v:IsA(‘Folder’) and convertInstanceToValue(v) or v.Value

As you can see these lines can get super lengthy, and lengthy lines aren’t good for readability. But when u have a short snippet of code and you are familiar with reading them, it’s perfectly readable.

  1. In your ‘verifyCurrentTable’ function, you seem to use the typeof function on a data type you commonly see in lua (not roblox lua / luau). typeof is meant to be used specifically on luau datatypes such as Vector3, etc… And it has been benchmarked, type is faster than typeof. So even if typeof does work, it’s still not the optimal solution.

  2. Your ‘toBoolean’ function is not useful at all. I wouldn’t recommend having string values called ‘true’ or ‘false in the first place. It’s just a waste of function calling. Even then, you can greatly shorten that function. You don’t need to use string.lower. And like I mentioned above, returning nil in a function is not necessary.

  3. In your ‘saveData’ function there’s is no need to concatenate the player’s UserID. The data store will work perfectly fine if u remove it. It’s a bad trend I see in many datastores.

3 Likes