I want to improve my DataStore code

Help and Feedback Code Review
#1
  • The code creates leaderstats with datastore.
  • The potential improvements I have considered are: shortening the code, making it work better and not debugging.
  • I especially want to improve on the code that it will be shorter

The code that I made:

local DataStore = game:GetService("DataStoreService")
local ds = DataStore:GetDataStore("CashSaveSystem")

game.Players.PlayerAdded:connect(function(player)
 local leader = Instance.new("Folder",player)
 leader.Name = "leaderstats"
 local Cash = Instance.new("IntValue",leader)
 Cash.Name = "Money"
 Cash.Value = ds:GetAsync(player.UserId) or 0
 ds:SetAsync(player.UserId, Cash.Value)
 Cash.Changed:connect(function()
  ds:SetAsync(player.UserId, Cash.Value)
 end)
end)


game.Players.PlayerRemoving:connect(function(player)
 ds:SetAsync(player.UserId, player.leaderstats.Cash.Value)
end)
#2

Remember that shorter code doesn’t necessarily mean more performant. This is about as performant as this code will get.

Now, if this is more of a question of style or readability than performance; then please get back to me so I can give a few points (although, all of them will be subjective as code style is. The code snippet provided isn’t “bad” by any means).

5 Likes
#3

Most likely I would scope the datastore onplayeradded

1 Like
#4

Looking at your current code, you have much more to worry about before considering to make it shorter.

  1. Usage of Instance.new()

Refer to this:

  1. No Retry or Fail Safe Mechanism

Utilize pcalls so you can check whether the datastore services are operational or not, otherwise you are risking complete data loss.

  1. Don’t use SetAsync to update player data

SetAsync is a destructive function. UpdateAsync will allow you to backup player data etc.

  1. Uncontrolled Data Saving

It’s not a good practice to update player data every time a change is made. You should update data in intervals otherwise you’ll hit datastore rate limits pretty quickly.

3 Likes
#5

Wow thx @wevetments. I really appreciate it

#6
  1. Use :Connect not :connect
  2. Set the parent of your created instances after changing their properties. The second argument of Instance.new() should only be used when you’re not going to set any properties.
  3. You do not have to constantly save a new value to the datastore when the cash is changed, you can just save it into a table then save the data when the player leaves. Datastore calls should use pcall or they can error.
  4. SetAsync can be used but if you need to worry about the previous value, you can use UpdateAsync.
#7

Use the game:BindToClose function to check if a game is shutting down. In that function, you can save the player’s data.

And also, try using UpdateAsync. It’s better and it can help prevent datastore failures.

#8

One thing I would change is not setting the data store every time the cash is changed. Data store limits can make players’ data not save.

Instead, use DataStore2. It prevents data loss. The creator has not experienced one data loss using DataStore2.

Also,

#9

Make players a service, don’t use deprecated functions. Use :Connect instead of :connect. Always parent last, not first since parenting it first is an inefficient way.

Shorter code = less bytes. It may be good to have shorter code but don’t expect some performance boost on the code unless you are dumping down from 800-4000 letters of code to 100-200.

Your CPU’s clock speed is also responsible for how fast it reads the code.

Example:

What to do:

local exampleFolder = Instance.new("Folder")
exampleFolder.Parent = player

What NOT to do:

local exampleFolder = Instance.new("Folder, player")