What's the best way to prevent my class from being a god class?

Help and Feedback Scripting Support
,
1

I’m working on a game new for practice, and I’ve decided to use OOP for the plot system and other things, and just wanted to know how you guys would prevent my plot class from being a god class? I feel like it’s doing too much. Should I instead break down some of the things into classes as well like the cash collector is a class, plot door is a class, and then i just call methods on those classes from the plot class?

Here’s my Plot class:

--// Services
local ServerScriptService = game:GetService("ServerScriptService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local PhysicsService = game:GetService("PhysicsService")

--// Modules
local modules = game.ServerScriptService:WaitForChild("Modules")
local NotificationService = require(modules:WaitForChild("NotificationService"))
local Zone = require(ServerScriptService.Libs:WaitForChild("Zone"))

--// Classes
local classes = ServerScriptService:WaitForChild("Classes")
local Animal = require(classes:WaitForChild("Animal"))

--// Libs
local Signal = require(ServerScriptService.Libs:WaitForChild("Signal"))

--// Steal Handler
local StealHandler = require(ServerScriptService.Modules.StealHandler)

--// Remotes
local remotes = ReplicatedStorage:WaitForChild("Remotes")
local playClientSound = remotes:WaitForChild("PlayClientSound")

--// Plot class
local Plot = {}
Plot.__index = Plot


function Plot.new(plotModel)
	local self = setmetatable({}, Plot)
	
	self.owner = nil
	self.model = plotModel
	
	self.animalLimit = 10
	self.animals = {}
	
	self.cashToCollect = 100
	self.collectDebounce = false
	
	--// Signals
	self.AnimalAdded = Signal.new() --// Fires when an animal is added to the plopt
	self.AnimalRemoved = Signal.new() --// Fires when an animal is removed from the plot
	self.PlayerEnteredPlot = Signal.new() --// Fires when any player enters the plot
	self.AnimalCountChanged = Signal.new() --// Fires when the animal count on the plot changes
	self.CollectCashButtonTouched = Signal.new() --// Fires when the collect cash button is touched by the owner of the plot
	self.AnimalLimitReached = Signal.new() --// Fires when the animal limit is reached on the plot
	
	--// Setup the base gate
	self.gate = self.model:FindFirstChild("Gate")
	self.locked = false
	self.lockDuration = 30
	
	local lockButtonModel = self.model:FindFirstChild("LockBaseButton")
	if lockButtonModel then
		local lockButton = lockButtonModel:FindFirstChild("LockButton")
		if lockButton then
			lockButton.Touched:Connect(function(hit)
				local player = game.Players:GetPlayerFromCharacter(hit.Parent)
				if player and player == self.owner then
					self:LockGate()
				end
			end)
		end
	end
	
	--// Setup cash to collect button and GUI
	local collectCashButton = self.model:FindFirstChild("CollectCashButton")
	if collectCashButton then
		self.collectPart = collectCashButton:FindFirstChild("CollectPart")
		
		local cashGui = self.collectPart:FindFirstChild("CashToCollectGui")
		if cashGui then
			local label = cashGui.Frame:FindFirstChild("CashToCollectLabel")
			if label then
				self.cashLabel = label
				self:UpdateCashLabel()
			end
		end
		
		if self.collectPart then
			self.collectPart.Touched:Connect(function(hit)
				local player = game.Players:GetPlayerFromCharacter(hit.Parent)
				if player and player == self.owner and self.cashToCollect > 0 and not self.collectDebounce then
					local amount = self.cashToCollect
					self.cashToCollect = 0
					self:UpdateCashLabel()
					self.CollectCashButtonTouched:Fire(player, amount)
					
					playClientSound:FireClient(player, "CashCollect", 1)
					
					task.delay(0.5, function()
						self.collectDebounce = false
					end)
				end
			end)
		end
		
	end
	
	--// setup land and animal count gui
	if self.model:FindFirstChild("Land") then
		self.land = self.model.Land
		
		-- Get animal count gui
		local animalCountGui = self.land:FindFirstChild("AnimalCountGui")
		if animalCountGui then
			self.animalCountLabel = animalCountGui:WaitForChild("CountLabel")
		end
	else
		warn("No land found in plot model:", self.model.Name)
	end
	
	--// setup the PlotZone
	local plotZonePart = self.model:FindFirstChild("PlotZone")
	if plotZonePart then
		self.plotZone = Zone.new(plotZonePart)
	end
	
	--// When a player entered the plot zone fire an event
	self.plotZone.playerEntered:Connect(function(player)
		if player == self.owner then
			self.PlayerEnteredPlot:Fire(player)
		end
	end)
	
	return self
end

function Plot:AddCash(amount)
	self.cashToCollect += amount
	self:UpdateCashLabel()
end

function Plot:UpdateCashLabel()
	if self.cashLabel and self.cashLabel:IsA("TextLabel") then
		self.cashLabel.Text = "$" .. tostring(self.cashToCollect)
	end
end

function Plot:GetAnimalCount()
	local count = 0
	for _ in pairs(self.animals) do
		count += 1
	end
	
	return count
end

function Plot:AddAnimalToData(animal)
	self.animals[animal.id] = animal
	self.AnimalCountChanged:Fire(self:GetAnimalCount(), self.animalLimit)
end

function Plot:RemoveAnimalFromData(animal)
	self.animals[animal.id] = nil
	self.AnimalCountChanged:Fire(self:GetAnimalCount(), self.animalLimit)
end

function Plot:IsFull()
	return self:GetAnimalCount() >= self.animalLimit
end

function Plot:UpdateAnimalCount()
	local count = self:GetAnimalCount()
	self.AnimalCountChanged:Fire(count, self.animalLimit)
end

function Plot:Claim(player)
	if not player then 
		return false
	end
	
	if self.owner ~= nil then
		return false
	end
	
	self.owner = player
	
	--// Setup owner sign
	local ownerSign = self.model:FindFirstChild("OwnerSign")
	if ownerSign then
		local ownerSignGui = ownerSign:FindFirstChild("OwnerSignGui")
		if ownerSignGui then
			local frame = ownerSignGui:FindFirstChild("Frame")
			local playerImageLabel = frame:FindFirstChild("PlayerImageLabel")
			local ownerNameLabel = frame:FindFirstChild("OwnerNameLabel")

			if frame and playerImageLabel and ownerNameLabel then
				print(self.owner.Name)
				ownerNameLabel.Text = self.owner.Name .. "'s Farm"
				playerImageLabel.Image = "rbxthumb://type=AvatarHeadShot&id=" .. self.owner.UserId .. "&w=420&h=420"
			end
		end
	end
	
	return true
end

function Plot:Unclaim()
	self.owner = nil
end

function Plot:IsOwner(player)
	return self.owner == player
end

function Plot:ClearPlot()
	--// Remove all animals from the plot
	for animalId, animal in pairs(self.animals) do
		self:RemoveAnimal(animal)
	end

	self:UpdateAnimalCount()
	self.owner = nil

	print("PLOT CLEARED")
end

function Plot:AddAnimal(animalName)
	if self:GetAnimalCount() >= self.animalLimit then
		self.AnimalLimitReached:Fire()
		return false
	end
	
	local newAnimal = Animal.new(animalName, self)
	if not newAnimal then return false end
	
	--// Set the animal to the plot position
	local plotAnimalsFolder = self.model:FindFirstChild("Animals")
	if not plotAnimalsFolder then
		warn("Plot animals folder not found couldnt spawn animal on plot")
		return
	end
	
	newAnimal.model.Parent = plotAnimalsFolder
	newAnimal.model:PivotTo(self.land:GetPivot() + Vector3.new(0, newAnimal.model.PrimaryPart.Size.Y / 2 + 0.3, 0))
	
	self.animals[newAnimal.id] = newAnimal
	self:UpdateAnimalCount()
	
	if self.AnimalAdded then
		self.AnimalAdded:Fire(newAnimal)
	end
	
	print("Added animal:", newAnimal.name, "with ID:", newAnimal.id)
	return true
end

function Plot:RemoveAnimal(animalId)
	local animal = self.animals[animalId]
	if animal then
		
		--// Stop animal behaviors and remove from state
		animal:StopEarning()
		animal:Destroy()
		
		self.animals[animalId] = nil
		
		self.AnimalRemoved:Fire(animal)
		self:UpdateAnimalCount()
	end
end

--// Gate stuff
function Plot:LockGate()
	if not self.gate then return end
	
	self.locked = true
	
	self.gate.Transparency = 0.5
	
	task.delay(self.lockDuration, function()
		self.locked = false
		self.gate.Transparency = 1
	end)
end

function Plot:SerializeData()
	
	--// Serialize animal data
	local animalsData = {}
	for _, animal in pairs(self.animals) do
		table.insert(animalsData, animal:Serialize())
	end
	
	return {
		Animals = animalsData,
		CashToCollect = self.cashToCollect,
	}
end

function Plot:LoadData(data)
	--// Load the animals back in
	self.animals = {}
	
	for _, animalData in ipairs(data.Animals or {}) do
		task.spawn(function()
			self:AddAnimal(animalData.Name, self)
			task.wait(0.1) -- // Temp remove this later after fixing client ready issues :O
		end)
	end
	
	--// Load cash to collect
	self.cashToCollect = data.CashToCollect or 0
	self:UpdateCashLabel()
	
	--// Refresh the animal count gui
	self:UpdateAnimalCount()
end

return Plot
2

The best strategy tends to suggest itself. There are a few different reasons why might break some code out of a large class and into its own module, including:

  • Organization: You have a lot of related code that is making exclusive use of some set of data members in the larger class, e.g. all your cash collecting code that has data members that only the cash collecting code uses
  • De-coupling: You might have a section of code that you intend to modify or re-factor, and you want to isolate it from the larger code base so that it can maintain a backwards-compatible API but internally be reworked and re-implemented.
  • Generalization: Suppose right now your plots all have animals, but in the future you want to add industrial plots that have only machines, or something else. It would make sense for the more general plot code to live apart from the code for specific plot subtypes. This can be done with either inheritance or composition, but in either case you end up with the code in separate modules.
  • Your class includes code that other classes could use: If your plot class had a bunch of helper functions to do finance calculations, or geometry, or something else general enough that you might want to re-use the code somewhere else, it might make sense to break it out into a function library module.
1 Like