Code Review for Data handler using ProfileStore

Help and Feedback Code Review
,
#1

This module is supposed to handle the data and ban data of players using ProfileStore (updated version of ProfileService), I am not satisfied with the difficulty of reading the code. I have already moved repeating code to helper functions but the code is still rather hard to read, would anyone be willing to provide help of making it more readable and/or optimized?

local dataHandler = {}

-- services
local RunService = game:GetService("RunService")
local Players = game:GetService("Players")
local SSS = game:GetService("ServerScriptService")

-- modules
local Modules = SSS.Modules
local ProfileStore = require(Modules.ProfileStore)

-- constants
local DATA_TEMPLATE = {
	money = 0,
	isBanned = false,
}
local BAN_TEMPLATE = {
	timesBanned = 0,
	history = {},
	currentBan = {},
}

-- initialize data
local PlayerData = ProfileStore.New("PlayerData", DATA_TEMPLATE)
local PlayerBanData = ProfileStore.New("BanData", BAN_TEMPLATE)

-- use mock in studio, not live data
if RunService:IsStudio() then
	PlayerData = PlayerData.Mock
end

-- active profiles
local Profiles = {}
local BanProfiles = {} -- cache for ban data profiles

-- helper functions
local function cleanupSessions(plr)
	local profile = Profiles[plr]
	local banCache = BanProfiles[plr]
	
	if banCache then
		banCache:EndSession()
		banCache[plr] = nil
	end
	if profile then
		profile:EndSession()
		Profiles[plr] = nil
	end
end

local function safelySaveProfile(profile)
	if profile then
		local success, err = pcall(function()
			profile:Save()
		end)
		
		if not success then
			warn("failed to save:", err)
		end
	end
end

local function isValidPlayer(plr)
	return plr and plr.Parent == Players
end


-- module functions

-- loads player data and ban data
function dataHandler:loadData(plr)
	if not isValidPlayer(plr) then return end

	-- loads main profile
	local profile = PlayerData:StartSessionAsync(tostring(plr.UserId), {
		Cancel = function()
			return not isValidPlayer(plr)
		end,
	})
	
	if profile then
		profile:AddUserId(plr.UserId)
		profile:Reconcile()

		profile.OnSessionEnd:Connect(function()
			Profiles[plr] = nil
			plr:Kick("profile session ended, please rejoin.")
		end)

		if isValidPlayer(plr) then
			Profiles[plr] = profile
			print("loaded player profile: "..plr.Name)
		else
			profile:EndSession()
		end
	else
		plr:Kick("data couldnt be loaded, rejoin")
	end
end

-- saves data
function dataHandler:saveData(plr)
	local profile = Profiles[plr]
	if profile then
		safelySaveProfile(profile)
		print("Data saved for " .. plr.Name)
	end
end

-- saves data and cleans up sessions
function dataHandler:saveDataOnLeave(plr)
	self:saveData(plr)
	cleanupSessions(plr)

	print(plr.Name .. " left, data saved")
end

-- saves all profiles
function dataHandler:saveAllProfiles()
	for plr in pairs(Profiles) do
		self:saveData(plr)
	end
end

-- gets profile of player
function dataHandler:getProfile(plr)
	return Profiles[plr]
end

-- gets bandata
function dataHandler:getBanData(plr)
	-- loads ban profile
	if BanProfiles[plr] then
		return BanProfiles[plr].Data
	end
	
	local banProfile = PlayerBanData:StartSessionAsync(tostring(plr.UserId), {
		Cancel = function()
			return not isValidPlayer(plr)
			end,
	})

	if banProfile then
		banProfile:Reconcile()
		banProfile:AddUserId(plr.UserId)
		BanProfiles[plr] = banProfile
		print("Ban data loaded for "..plr.Name)
		return banProfile
	else
		warn("couldnt get ban data for "..plr.Name)
	end
end

return dataHandler
#2

If there is one thing that would increase code readability with minimal effort is to transform conditions to avoid nesting. I would argue this is one of, if not the best, method for instantly improving code quality.

For instance, take the function dataHandler:getBanData. You can transform the conditions to remove unnecessary nesting.

function dataHandler:getBanData(plr)
    if BanProfiles[plr] then return BanProfiles[plr].Data end

    --...

    if not banProfile then warn("...") end

    banProfile:Reconcile()
    banProfile:AddUserId(plr.UserId)
    banProfiles[plr] = banProfile
    print("...")

    return banProfile

end

The conditions have either been put onto a single line to remove unnecessary nesting, or been flipped to avoid having to nest the following code after it (or in this case, both in a single condition). The whole point of avoiding nesting is to prevent having to keep looking up and down a block of code to remember the fail conditions. If the fail conditions are at the very top of the function, you can ignore them and keep your focus on the main body of code. This is especially significant for easier debugging.

Next, you should consider following variable naming schemes. Your variables, constants, and function names are very inconsistent and will definitely make it harder to differentiate the three types whilst programming.

Variables should follow the camelCase naming scheme.
thisIsTheCamelCaseNamingScheme

Constants should follow the SCREAMING_SNAKE_CASE naming scheme.
THIS_IS_THE_SCREAMING_SNAKE_CASE`_NAMING_SCHEME

Functions should follow the PascalCase naming scheme.
ThisIsThePascalCaseNamingScheme

Your code correctly features examples of these naming schemes, however, it is not consistent throughout.