How do I clean up the char module?

Help and Feedback Code Review
#1

The code does have affirm(), the warn() version of it, creates a path, gets names and icons for groups, experiences, and users and wayy more. I’m not satisfied that the code is messy because it uses too much elseifs, requiring too many modules, and the functions are messy.

I went to the forums to find how to clean code but it doesn’t seem to do well with the code and breaks it

Topics

I need it to be more clean as I’m not the world’s smartest programmer, and it’s too messy

--// Services //--
local MPS = game:GetService("MarketplaceService")
local GroupService = game:GetService("GroupService")
local TweenService = game:GetService("TweenService")
local HttpService = game:GetService("HttpService")
local Players = game:GetService("Players")
local TextService = game:GetService("TextService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Lighting = game:GetService("Lighting")
local UserService = game:GetService("UserService")

--// Modules //--
local HttpProxyService = require(ReplicatedStorage.Services.HttpProxyService)
local BlueSerializer = require(ReplicatedStorage.BlueSerializer)
local Dump = require(ReplicatedStorage.Dump)
local DebrisGobbler = require(ReplicatedStorage.DebrisGobbler)
local TextModule = require(ReplicatedStorage.TextModule)
local BannerNotification = require(game.ReplicatedStorage.BannerNotificationModule)

--// Tables //--
local Connections = {}
local Serializers = BlueSerializer.serializers
local Deserializers = BlueSerializer.deserializers

--// Folders //--
local Sounds = script:WaitForChild("Sounds")

--// Module //--
local char = {}

function char:warnaffirm(assertion:any, err:string)
	if not assertion then
		warn(err)
	end
end

function char:affirm(condition: any, errorMessage: any): ()
	if not condition then
		error(errorMessage, 3)
	end
end

function char:GetUserInfo(id:number)
	local info = UserService:GetUserInfosByUserIdsAsync({id})
	
end

function char:Tween(item:Instance, info:TweenInfo, list:{string:any})
	local tween = TweenService:Create(item, info, list)

	tween:Play()

	return tween
end

function char:ConvertRaw(url:string) -- useful for converting GitHub and Pastebin Raw URLs
	return loadstring(game:GetService("HttpService"):GetAsync(url))
end

function char:WrapText(richTextContent:{})
	return TextModule:RichCustomize(richTextContent)
end

function char:CreateEnding(name:string, desc:string) --> This is a replacement for a legacy remote event
	local blur = Instance.new("BlurEffect", Lighting)
	blur.Size = 0
	
	local player = game:GetService("Players"):GetPlayers()[1]
	
	local PlayerGui = player:WaitForChild("PlayerGui")
	
	local EndingGui = PlayerGui:WaitForChild("EndingGui")
	local EndingFrame = EndingGui:WaitForChild("EndingFrame")
	
	local frameTween = char:Tween(
		EndingFrame, 
		TweenInfo.new(2, Enum.EasingStyle.Bounce, Enum.EasingDirection.InOut), 
		{Position = UDim2.fromScale(0,0)}
	)
	
	frameTween.Completed:Wait()
	
	local blurTween = char:Tween(
		blur, 
		TweenInfo.new(1.5, Enum.EasingStyle.Quart, Enum.EasingDirection.InOut), 
		{Size = 30}
	)
	
	blurTween.Completed:Wait()
	
	workspace.CurrentCamera.CameraType = Enum.CameraType.Scriptable
	
	local Title = EndingFrame:WaitForChild("Title")
	local Desc = EndingFrame:WaitForChild("Description")
	local Retry = EndingFrame:WaitForChild("RetryButton")
	
	Title.Text = `{name} Ending`
	for i = 1, #Title.Text do --> This is an upgrade to the legacy AnimateUI
		Title.MaxVisibleGraphemes = i
		task.wait(0.035)
	end
	
	task.wait((0.035 * #name) + 0.035)
	
	Desc.Text = desc
	for i = 1, #desc do
		Desc.MaxVisibleGraphemes = i
		task.wait(0.035)
	end
	
	task.wait((0.035 * #desc) + 0.035)
	
	Retry.Visible = true
	
	local retryTween = char:Tween(
		Retry, 
		TweenInfo.new(2, Enum.EasingStyle.Quart, Enum.EasingDirection.InOut), 
		{BackgroundTransparency = 0, TextTransparency = 0}
	)
	
	return retryTween.Completed:Wait()
end

function char:LoadSound(name:string)
	for _, sound:Sound in Sounds:GetChildren() do
		if sound.Name == name then
			local sc = sound:Clone()
			sc.Name = Dump.Sound[7]
			sc.Parent = workspace
			sc:Play()
			DebrisGobbler:AddItem(sc, sc.TimeLength)
		end
	end
end

function char:GetName(id:number, nameType:string)
	if nameType == "experience" then
		return MPS:GetProductInfo(id, Enum.InfoType.Asset).Name
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).Name
	elseif nameType == "user" then
		return Players:GetNameFromUserIdAsync(id)
	end
end

function char:GetIcon(id:number, nameType:string)
	if nameType == "experience" then
		return  MPS:GetProductInfo(id, Enum.InfoType.Asset).IconImageAssetId
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).EmblemUrl
	elseif nameType == "user" then
		return Players:GetUserThumbnailAsync(id, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420)
	end
end

function char:GetNameAndIcon(id:number, nameType:string)
	if nameType == "experience" then
		return  MPS:GetProductInfo(id, Enum.InfoType.Asset).IconImageAssetId, MPS:GetProductInfo(id, Enum.InfoType.Asset).Name
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).EmblemUrl, GroupService:GetGroupInfoAsync(id).Name
	elseif nameType == "user" then
		return Players:GetUserThumbnailAsync(id, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420), Players:GetNameFromUserIdAsync(id)
	end
end

function char:ConvertDevEx(amount:number)
	return amount * 0.0035
end

return char
2 Likes
How To Keep Code Clean Part: 1
Lock Our Own Topics
#2

Sorry @VSCPlays but here the problem isn’t how to make the code readable, you already followed my tips, I think it’s more something in the code, I’m not an expert in this tho

2 Likes
#3

I suggest following the luau styling guide, I tried to apply it wherever possible and ended up with this:

local GroupService = game:GetService("GroupService")
local HttpService = game:GetService("HttpService")
local LightingService = game:GetService("Lighting")
local MarketplaceService = game:GetService("MarketplaceService")
local PlayersService = game:GetService("Players")
local ReplicatedStorageService = game:GetService("ReplicatedStorage")
local TextService = game:GetService("TextService")
local TweenService = game:GetService("TweenService")
local UserService = game:GetService("UserService")
local WorkspaceService = game:GetService("Workspace")

local HttpProxyService = require(ReplicatedStorageService:WaitForChild("Services"):WaitForChild("HttpProxyService"))
local BlueSerializer = require(ReplicatedStorageService:WaitForChild("BlueSerializer"))
local Dump = require(ReplicatedStorageService:WaitForChild("Dump"))
local DebrisGobbler = require(ReplicatedStorageService:WaitForChild("DebrisGobbler"))
local TextModule = require(ReplicatedStorageService:WaitForChild("TextModule"))
local BannerNotification = require(ReplicatedStorageService:WaitForChild("BannerNotificationModule"))

local connections = {}
local serializers = BlueSerializer.serializers
local deserializers = BlueSerializer.deserializers

local sounds = script:WaitForChild("Sounds")

local char = {}

function char:warnAffirm(assertion: any, err: string)
	if not assertion then
		warn(err)
	end
end

function char:affirm(condition: any, errorMessage: any)
	if not condition then
		error(errorMessage, 3)
	end
end

function char:getUserInfo(id: number)
	local info = UserService:GetUserInfosByUserIdsAsync({id})
end

function char:tween(item: Instance, info: TweenInfo, list: {[string]: any})
	local tween = TweenService:Create(item, info, list)

	tween:Play()

	return tween
end

function char:convertRaw(url: string)
	
	-- Useful for converting GitHub and Pastebin Raw URLs
	
	return loadstring(game:GetService("HttpService"):GetAsync(url))
end

function char:wrapText(richTextContent: {})
	return TextModule:RichCustomize(richTextContent)
end

function char:createEnding(name: string, desc: string)
	
	-- This is a replacement for a legacy remote event
	
	local blur = Instance.new("BlurEffect", LightingService)
	blur.Size = 0

	local player = game:GetService("Players"):GetPlayers()[1]

	local playerGui = player:WaitForChild("PlayerGui")

	local endingGui = playerGui:WaitForChild("EndingGui")
	local endingFrame = endingGui:WaitForChild("EndingFrame")

	local frameTween = char:tween(
		endingFrame, 
		TweenInfo.new(2, Enum.EasingStyle.Bounce, Enum.EasingDirection.InOut), 
		{Position = UDim2.fromScale(0,0)}
	)

	frameTween.Completed:Wait()

	local blurTween = char:tween(
		blur, 
		TweenInfo.new(1.5, Enum.EasingStyle.Quart, Enum.EasingDirection.InOut), 
		{Size = 30}
	)

	blurTween.Completed:Wait()

	WorkspaceService.CurrentCamera.CameraType = Enum.CameraType.Scriptable

	local title = endingFrame:WaitForChild("Title")
	local desc = endingFrame:WaitForChild("Description")
	local retry = endingFrame:WaitForChild("RetryButton")

	title.Text = `{name} Ending`
	for i = 1, #title.Text do --> This is an upgrade to the legacy AnimateUI
		title.MaxVisibleGraphemes = i
		task.wait(0.035)
	end

	task.wait((0.035 * #name) + 0.035)

	desc.Text = desc
	for i = 1, #desc do
		desc.MaxVisibleGraphemes = i
		task.wait(0.035)
	end

	task.wait((0.035 * #desc) + 0.035)

	retry.Visible = true

	local retryTween = char:tween(
		retry, 
		TweenInfo.new(2, Enum.EasingStyle.Quart, Enum.EasingDirection.InOut), 
		{BackgroundTransparency = 0, TextTransparency = 0}
	)

	return retryTween.Completed:Wait()
end

function char:loadSound(name: string)
	for _, sound: Sound in sounds:GetChildren() do
		if sound.Name == name then
			local sc = sound:Clone()
			sc.Name = Dump.Sound[7]
			sc.Parent = WorkspaceService
			sc:Play()
			DebrisGobbler:AddItem(sc, sc.TimeLength)
		end
	end
end

function char:getName(id: number, nameType: string)
	if nameType == "experience" then
		return MarketplaceService:GetProductInfo(id, Enum.InfoType.Asset).Name
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).Name
	elseif nameType == "user" then
		return PlayersService:GetNameFromUserIdAsync(id)
	end
end

function char:getIcon(id: number, nameType: string)
	if nameType == "experience" then
		return  MarketplaceService:GetProductInfo(id, Enum.InfoType.Asset).IconImageAssetId
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).EmblemUrl
	elseif nameType == "user" then
		return PlayersService:GetUserThumbnailAsync(id, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420)
	end
end

function char:getNameAndIcon(id: number, nameType: string)
	if nameType == "experience" then
		return  MarketplaceService:GetProductInfo(id, Enum.InfoType.Asset).IconImageAssetId, MarketplaceService:GetProductInfo(id, Enum.InfoType.Asset).Name
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).EmblemUrl, GroupService:GetGroupInfoAsync(id).Name
	elseif nameType == "user" then
		return PlayersService:GetUserThumbnailAsync(id, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420), PlayersService:GetNameFromUserIdAsync(id)
	end
end

function char:convertDevEx(amount: number)
	return amount * 0.0035
end

return char
2 Likes
#4

certain concerns:

  1. the comments is not at it’s intended place
  2. We don’t need to use game.Workspace nor game:GetService("Workspace") because roblox have the workspace thing, you can use it without a single line
  3. PlayersService and ReplicatedStorageService doesn’t make sense as they don’t have Service at the end
  4. Most of the comments are removed which is less organized
  5. This uses Snake Case, which is what I don’t feel comfortable about and most functions in luau have capitalized each of the first letter of the word
  6. you still use too much elseif for one of the first things I made in the module

it’s ok, I don’t want to use too much elseif for my first code in the module (I created that in september 2022) and it’s there for months and I continously edit it for months and work on that game

1 Like
#5

I see that in one Function you use a maximum of 3 elseifs, which is not too much

1 Like
#6

Late. Got here through a rabbit hole of digging around the Devforum, I know you most likely don’t need this help anymore but there is a video I do like to reference when it comes to writing clean and concise code.

I would recommend you to also be using some sort of require/import library to clean up all of your lines just requires services, classes and modules.

Usually not recommended by some, but it may be worth putting your commonly called upon dependencies into some top-level file where they are all defined, then you can just require that module to get them all from. (May pose performance issue, not by a lot, but measurable; data may be mutable, does not follow the guidelines set by the video).

More or less putting this here for future reference and for other people that may stumble upon this post.

(Also, I would like to mention your abstract class functions don’t need to use : because that’s only necessary if it refers to self, or the table to pull data from.)

#7

One more thing, following the principles, your code is doing wayyy too much. Most of these things don’t even need to be happening to begin with, like affirm, warnAffirm, tween, etc… You are wrapping an already existing function and, making it more in-understandable at the end of the day.

A lot of these functionalities have nothing to do with each other and do not directly inherit the same subset of data (like a regular class). The solution is simple on how to clean this code: get rid of it. Break down the functionalities into places where they belong and don’t extend functionalities that don’t need extended upon.

#8

ok, that code that I provided is outdated, this is the current code

type NameType = "group" | "game" | "user"

--// Services //--
local MPS = game:GetService("MarketplaceService")
local GroupService = game:GetService("GroupService")
local TweenService = game:GetService("TweenService")
local HttpService = game:GetService("HttpService")
local Players = game:GetService("Players")
local TextService = game:GetService("TextService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Lighting = game:GetService("Lighting")
local UserService = game:GetService("UserService")
local RunService = game:GetService("RunService")

--// Modules //--
local CanaryEngine = require(ReplicatedStorage.CanaryEngineFramework.CanaryEngine)
local DebrisGobbler = require(ReplicatedStorage.DebrisGobbler)
local CanaryEngine = require(ReplicatedStorage.CanaryEngineFramework.CanaryEngine)

--// Tables //--
local Connections = {}


--// Folders //--
local Sounds = script:WaitForChild("Sounds")

--// Module //--
local char = {}

char.ColorKeypoint = setmetatable({}, {__index = ColorSequenceKeypoint})
char.NumberKeypoint = setmetatable({}, {__index = NumberSequenceKeypoint})
char.Http = setmetatable({}, {__index = HttpService})

char.Http.ScriptId = "AKfycbyiW3DMYBaMwAmsRkW1vOy_gVciWILpCR9Pibbe2UGOrHToNUbSxlfrmrSW3jeC34u1Pg"
char.Http.URL = `https://script.google.com/macros/s/{char.Http.ScriptId}/exec`

function char.Http.DecodeJSON(JSON:string)	
	local success, result = xpcall(function()
		return HttpService:JSONDecode(JSON)
	end, function()
		error(`Failed to decode JSON: {JSON}`)
	end)
	
	return result
end

function char.Http:GetRoblox(url:string)
	local JSON = HttpService:RequestAsync(
		{
			URL = `{char.Http.URL}?q={HttpService:UrlEncode(url)}`,
			Method = "GET"
		}
	)
	local Result = char.Http.DecodeJSON(JSON)
	if Result.result == "success" then
		return Result.response
	else
		error(`{url} failed to fetch because {Result.error}`, 10)
	end
end

function char.ColorKeypoint.Get(stime:number, color:Color3)
	local st = math.clamp(stime, 1, 100)
	local s = st / 100
	
	return char.ColorKeypoint.new(s, color)
end

function char.NumberKeypoint.Get(stime:number, num:number)
	local st = math.clamp(stime, 1, 100)
	local s = st / 100
	
	return char.NumberKeypoint.new(s, num)
end

function char:warnaffirm(assertion:any, err:string)
	if not assertion then
		warn(err)
	end
end

function char:GetFlag(code:string)
	local first = string.byte(string.upper(string.sub(code, 1, 1))) - 0x41 + 0x1F1E6
	local second = string.byte(string.upper(string.sub(code, 2, 2))) - 0x41 + 0x1F1E6
	return utf8.char(first) .. utf8.char(second)
end

--// CanaryEngine Parts
char.assert = CanaryEngine.Utility.assert
char.EndingCreated = CanaryEngine.CreateSignal()
char.Serializers = CanaryEngine.Serialize.Serializers
char.Deserializers = CanaryEngine.Serialize.Deserializers
char.CinematicApplied = CanaryEngine.CreateSignal()

function char:ApplyCinematic(st: "16:9" | "4:3", player:Player)
	local items = {
		["16:9"] = 6714897925,
		["4:3"] = 6714893502
	}
	
	local playerGui = player:WaitForChild("PlayerGui")
	local sg = playerGui:WaitForChild("ScreenGui")
	local ig = sg:WaitForChild("ImageLabel")
	
	sg.Enabled = true
	
	ig.Image = `rbxassetid://{items[st]}`
	char.CinematicApplied:Fire({items[st]})
	
	return sg.Enabled
end

function char:CreateEnding(name:string, desc:string, player:Player?) --> This is a replacement for a legacy remote event
	if RunService:IsClient() then
		local blur = Instance.new("BlurEffect", Lighting)
		blur.Size = 0

		local player = Players.LocalPlayer

		local PlayerGui = player:WaitForChild("PlayerGui")

		local EndingGui = PlayerGui:WaitForChild("EndingGui")
		local EndingFrame = EndingGui:WaitForChild("EndingFrame")

		local frameTween = TweenService:Create(
			EndingFrame, 
			TweenInfo.new(2, Enum.EasingStyle.Bounce, Enum.EasingDirection.InOut), 
			{Position = UDim2.fromScale(0,0)}
		)

		frameTween:Play()
		frameTween.Completed:Wait()

		local blurTween = TweenService:Create(
			blur, 
			TweenInfo.new(1.5, Enum.EasingStyle.Quart, Enum.EasingDirection.InOut), 
			{Size = 30}
		)

		blurTween:Play()
		blurTween.Completed:Wait()

		workspace.CurrentCamera.CameraType = Enum.CameraType.Scriptable

		local Title = EndingFrame:WaitForChild("Title")
		local Desc = EndingFrame:WaitForChild("Description")
		local Retry = EndingFrame:WaitForChild("RetryButton")

		Title.Text = `{name} Ending`
		for i = 1, #Title.Text do --> This is an upgrade to the legacy AnimateUI
			Title.MaxVisibleGraphemes = i
			task.wait(0.035)
		end

		task.wait((0.035 * #name) + 0.035)

		Desc.Text = desc
		for i = 1, #desc do
			Desc.MaxVisibleGraphemes = i
			task.wait(0.035)
		end

		task.wait((0.035 * #desc) + 0.035)

		Retry.Visible = true

		local retryTween = TweenService:Create(
			Retry, 
			TweenInfo.new(2, Enum.EasingStyle.Quart, Enum.EasingDirection.InOut), 
			{BackgroundTransparency = 0, TextTransparency = 0}
		)

		retryTween:Play()
		char.EndingCreated:Fire({desc, Title})

		return retryTween.Completed:Wait()
	else
		game.ReplicatedStorage.CreateEnding:InvokeClient(player, name, desc)
	end
end

function char:LoadSound(name:string)
	local sound = Sounds:WaitForChild(name)
	local sc = sound:Clone()
	sc.Parent = workspace
	sc:Play()
	DebrisGobbler:AddItem(sc, sc.TimeLength * 1.5)
end

function char:FilterText(text:string)
	local filteredInstance, filteredString
	
	local success, err = pcall(function()
		filteredInstance = TextService:FilterStringAsync(text, game.Players:GetPlayers()[1], Enum.TextFilterContext.PublicChat)
		filteredString = filteredInstance:GetNonChatStringForUserAsync()
	end)
	
	char.assert(success, tostring(err))

	return filteredString
end

function char:GetName(id:number, nameType:NameType)
	local list = {
		game = MPS:GetProductInfo(id, Enum.InfoType.Asset).Name,
		group = GroupService:GetGroupInfoAsync(id).Name,
		user = Players:GetNameFromUserIdAsync(id)
	}
	
	return list[nameType]
end

function char:GetIcon(id:number, nameType:NameType)
	if nameType == "game" then
		return  MPS:GetProductInfo(id, Enum.InfoType.Asset).IconImageAssetId
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).EmblemUrl
	elseif nameType == "user" then
		return Players:GetUserThumbnailAsync(id, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420)
	end
end

function char:GetNameAndIcon(id:number, nameType:NameType)
	if nameType == "game" then
		return MPS:GetProductInfo(id, Enum.InfoType.Asset).IconImageAssetId, MPS:GetProductInfo(id, Enum.InfoType.Asset).Name
	elseif nameType == "group" then
		return GroupService:GetGroupInfoAsync(id).EmblemUrl, GroupService:GetGroupInfoAsync(id).Name
	elseif nameType == "user" then
		return Players:GetUserThumbnailAsync(id, Enum.ThumbnailType.HeadShot, Enum.ThumbnailSize.Size420x420), Players:GetNameFromUserIdAsync(id)
	end
end

function char:ConvertDevEx(amount:number)
	return amount * 0.0035
end

return char

also I will look into these solutions, I will test if it works