Help on Where to Go with This Class Constructor

Help and Feedback Code Review
#1

This is a class I’m using to manage player datastores. I’m not using metatables because I want the “intellisense” to be available in other files. Something I want to know is how should I name my methods? It’s obviously a PlayerData type so should I drop the “Data” part that’s in all these methods? Also, I have the method incrementData() , should I also include decrementData() just to be explicit? My final question is if memory usage is going to be a problem because the methods are inside the constructor rather than the usual outside of constructor plus metatable combo. Let me know any other thoughts you have on this, thanks.

-- note this isn't the exact code
type PlayerData = {
 -- all the variable and methods would be in here
}
-- constructor
function PlayerData.new(player: Player): PlayerData
	local this = {}
    this.publicUtilTable
	this._private
    this._otherPrivate

	function this.getData(dataName: string?)

	end
	-- sends the client their data via remote event
	function this.sendData() 

	end
	
	function this.setData(key: string? | any, value: any?)
        
	end
	
	function this.incrementData(key: string, value: number)

	end
	
-- similar to getData except it returns a reference to the player's data instead of potentially returning a copy
	function this._getRef()
       
	end

	function this.resetData()
		
	end
	
	return this
end
1 Like
#2

I would drop the data from the name of the functions if you plan to keep it how it is (more on this in a sec)

I wouldn’t, I would just leave a comment above incrementData() saying you are free to use negatives. My reasioning is that adding the function doesn’t really add to the code or do anything that different that we would really need it.

It is something to keep in mind if you are saving it outside of the scope like inserting it into a different module or a global variable.
That being said if it were me, instead of making an object specific to a player, I would make it stateless by making it it’s own module:

local module = {}

function module.getData(player :Player, dataName :string) end
-- sends the client their data via remote event
function module.sendDataToClient(player :Player)  end

function module.setData(player :Player, key: string, value :any) end

function module.incrementData(player :Player, key :string, value :number) end

-- similar to getData except it returns a reference to the player's data instead of potentially returning a copy
function module._getRef(player :Player) end

function module.resetData(player :Player) end

return module

Another thing to note: string? | any
any is litterally anything including nil
when you say ? you are saying it could be a string or nil string | nil
this type should just be shortened to string?
string? == string|nil

I am a very procedural type of programmer so it’s nice to see someone trying out different classes and stuff. Looks like you though things out pretty well

2 Likes