What can I add to my code to make it better?

Help and Feedback Code Review
#1

I basically have made a wrapper for BridgeNet, a networking solution, and I don’t what else I could add. I feel like it’s really empty and could use something else.

The module:

-- // Variables

local Plugins = script.Plugins
local Assets = script.Assets
local Settings = require(script.Settings)

---

local RunService = game:GetService("RunService")

local BridgeNet = require(Plugins.BridgeNet)

local TakenEventNames = { }

local NetworkSignal = { }
local Signal = { }

local Connection = { }
local ConnectionMethods = { }

local Local = RunService:IsClient()
local Server = RunService:IsServer()

local LocalError = "Cannot fire to clients on the client."
local ServerError = "Cannot fire to server on the server."

-- // Functions

-- // // Connection Class

function Connection.new(event: any)
	local RBXScriptConnection = setmetatable({ }, {__index = ConnectionMethods})

	RBXScriptConnection.Connected = true :: boolean
	RBXScriptConnection._event = event

	return RBXScriptConnection
end

function ConnectionMethods:Disconnect()
	self.Connected = false
	self._event.Disconnect()
end

-- // // Signal Class

function NetworkSignal.new(name: string)
	if table.find(TakenEventNames, name) then
		error(string.format("ID: '%s' was already created.", name))
	end
	
	table.insert(TakenEventNames, name)
	
	return setmetatable({
		_networksignal = BridgeNet.CreateBridge(name)
	}, {__index = Signal})
end

function Signal:Connect(func: (...any) -> ())
	local connected = self._networksignal:Connect(func)
	
	return Connection.new(connected)
end

function Signal:FireServer(...: any)
	if not Server then
		self._networksignal:Fire(...)
	else
		error(ServerError, 0)
	end
end

function Signal:FireClient(player: Player, ...: any)
	if not Local then
		self._networksignal:FireTo(player, ...)
	else
		error(LocalError, 0)
	end
end

function Signal:FireAllClients(...: any)
	if not Local then
		self._networksignal:FireAll(...)
	else
		error(LocalError, 0)
	end
end

function Signal:FireClients(players: {Player}, ...: any)
	if not Local then
		self._networksignal:FireToMultiple(players, ...)
	else
		error(LocalError, 0)
	end
end

function Signal:Once(func: (...any) -> ())
	self._networksignal:Once(func)
end

function Signal:Destroy()
	self._networksignal:Destroy()
	self = nil
end

return NetworkSignal

What I was trying to achieve was something similar to @stravant’s Signal module. Anyways, I would like to hear your thoughts.

#2

There are a few things that could be added to make this code better:

  1. Add type annotations to the function arguments and return types. This will make the code easier to read and understand, and can also help catch errors at compile time.
  2. Add documentation comments to each function explaining what it does and how to use it. This will make it easier for other developers to understand and use the code.
  3. Add error handling for when the input arguments are invalid or unexpected. For example, the FireClient function should check that the player argument is a valid Player object, and the FireClients function should check that the players argument is a table of Player objects.
  4. Consider refactoring the code to use classes and inheritance, rather than using metatables. This can make the code more organized and easier to understand, especially if the codebase is large or complex.
  5. Consider using a linter or static analysis tool to automatically check for common issues and style violations. This can help catch mistakes and enforce best practices. here is the script
--[[
    NetworkSignal is a module that provides a simple interface for
    firing and handling events across the client/server boundary.
    
    To use this module, create a new NetworkSignal with a unique name:
    
    local signal = NetworkSignal.new("MySignalName")
    
    Then, use the `Connect` method to register a callback for the signal:
    
    local connection = signal:Connect(function(...)
        print("MySignalName was fired with arguments: ", ...)
    end)
    
    To fire the signal from the server, use the `FireServer` method:
    
    signal:FireServer(arg1, arg2, arg3)
    
    To fire the signal to a specific client, use the `FireClient` method:
    
    signal:FireClient(player, arg1, arg2, arg3)
    
    To fire the signal to all clients, use the `FireAllClients` method:
    
    signal:FireAllClients(arg1, arg2, arg3)
    
    To fire the signal to a group of clients, use the `FireClients` method:
    
    signal:FireClients({player1, player2, player3}, arg1, arg2, arg3)
    
    To register a callback that is only called once, use the `Once` method:
    
    signal:Once(function(...)
        print("MySignalName was fired with arguments: ", ...)
    end)
    
    To unregister a callback, use the `Disconnect` method on the connection object:
    
    connection:Disconnect()
    
    To destroy the signal, use the `Destroy` method:
    
    signal:Destroy()
]]

local Plugins = script.Plugins
local Assets = script.Assets
local Settings = require(script.Settings)

---

local RunService = game:GetService("RunService")

local BridgeNet = require(Plugins.BridgeNet)

local TakenEventNames = { }

local NetworkSignal = { }
local Signal = { }

local Connection = { }
local ConnectionMethods = { }

local Local = RunService:IsClient()
local Server = RunService:IsServer()

local LocalError = "Cannot fire to clients on the client."
local ServerError = "Cannot fire to server on the server."

-- // Functions

-- // // Connection Class

function Connection.new(event: RBXScriptConnection)
    local RBXScriptConnection = setmetatable({ }, {__index = ConnectionMethods})

    RBXScriptConnection.Connected = true
    RBXScriptConnection._event = event

    return RBXScriptConnection
end

function ConnectionMethods:Disconnect()
    self.Connected = false
    self._event:Disconnect()
end

-- // // Signal Class

function NetworkSignal.new(name: string)
    if table.find(TakenEventNames, name) then
        error(string.format("ID: '%s' was already created.", name))
    end
    
    table.insert(TakenEventNames, name)
    
    return setmetatable({
        _networks
1 Like