Database Module

Help and Feedback Code Review
#1

Sorry if this is mis-formatted, just based off of how the template states it should be formatted.

The below code is a datastore/database module that makes my code a little cleaner - I’m not necessarily dissatisfied, but I just want any input as to what I should change (if anything)
Any optimizations or improvements to readability I have at least attempted, most working fine.

However, if you guys have any optimizations (or readability improvements) in mind, that’d be great!

local print, warn, error = nil;
local serverscriptservice = game:GetService("ServerScriptService");
local logger = require(serverscriptservice.modules.server_logger):new();
print, warn, error =
	function(str : string) logger:logInformation(str) end,
	function(str : string) logger:logWarning(str) end,
	function(str : string) logger:logError(str) end

local db = {};
db.__index = db;

db.new = function(self : typeof(db), db_name : string?)
	if not db_name then
		error("Too few parameters in method db:new()");
		return;
	end
	self = setmetatable({}, db);
	self.Datastore = game:GetService("DataStoreService"):GetDataStore(db_name);
	print(`Successfully retrieved datastore {db_name}`);
	return self;
end

db.Save = function(self : typeof(db), user_id : number?, data : {}?)
	if not user_id or not data then
		error("Too few parameters in method db:Save()");
		return;
	elseif type(data) ~= "table" then
		error("Data is not in table format (db:Save() only takes table parameters for parameter 'data')");
		return;
	elseif type(user_id) ~= "number" then
		error(`User ID must be a number, not a {type(user_id)}!\n\t\t\t\tmethod: db:Save()`);
		return;
	end
	
	local success, exception = pcall(function()
		self.Datastore:SetAsync(user_id, data);
	end)
	if not success then
		error(`Failed to save data for {user_id}\nException: {exception}`);
		return;
	end
	print(`Successfully saved data for {user_id}`);
end

db.Retrieve = function(self : typeof(db), user_id : number?)
	if not user_id then
		error("Too few paramters in method db:Save()");
		return;
	elseif type(user_id) ~= "number" then
		error(`User ID must be a number, not a {type(user_id)}!\method: db:Retrieve()`);
		return;
	end
	
	local success, data = pcall(function()
		return self.Datastore:GetAsync(user_id);
	end)
	if success then
		print(`Successfully retrieved data for {user_id}`);
		return data;
	end
	error(`Failed to retrieve data for {user_id}`)
end

db.ClearIndex = function(self : typeof(db), user_id : number?)
	if not user_id then
		error("Too few paramters in method db:Save()");
		return;
	elseif type(user_id) ~= "number" then
		error(`User ID must be a number, not a {type(user_id)}!\method: db:ClearIndex()`);
		return;
	end
	
	local success, exception = pcall(function()
		self.Datastore:RemoveAsync(user_id);
	end)
	if not success then
		error(`Failed to delete data for {user_id}\nException: {exception}`);
		return;
	end
	print(`Successfully cleared data for {user_id}`);
end

return db;

functions:
db:new() db:Save() db:Retrieve() db:ClearIndex()

the logger module just appends [ERROR], [WARN]. and [INFO], just for the record

#2

image
began checking lines, added a few missed \n and \t

#3

You don’t need to pass self to dB.new(), partly because you overwrite self a couple lines later, but also this is the constructor function so you shouldn’t already have a db metatable set to the object you are passing.

If you use : instead of . it passes self automatically.

You can define the type as type db = typeof(db.new("") after the constructor function

#4

Tested the self : typeof(db), I do in fact need it - i get an error about something like “tables can not be cyclic” among a few other warnings.

Defining the type? What exactly does that mean?

#5

You would need to structure it slightly differently.

self : typeof(dB.new())

This will cast the type as the type of table returned from the constructor (not the table you are using as the metatable).

I just mean instead of writing self:typeof(blah) you could write

export type database = typeof(dB.new())

Then each time you need to specify a parameter/variable’s type you can write instead

var : database

The export keyword lets you use this type in any script which has required the module for example

local dbModule = require(path)
local db : dbModule.database = dbModule.new()
#6

Hiya, so the way he has done it will work perfectly fine as I taught him to do it this way, and I have a decade of experience. They simply just did not understand at the time and I’ve since corrected them.