Am I overusing OOP Getters?

I’m working on a inventory, and in my classes I’m using getter methods (a lot of them)

Example 1
function Inventory:GetSlots()
	return self.Slots
end

function Inventory:GetSlot(key)
   return self:GetSlots()[key]
end

function Inventory:GetInventoryData()
	return self.inventoryData
end

in use

function Inventory:AddSlotAtKey(key, slot)
	assert(slot.Class == "Slot", "expected Slot for arg #2")
	local existingSlot = 
	if not self:GetSlot(key)then
		self:GetSlots()[key] = slot
		self:OnSlotChanged(key, nil, slot)
	end
end
Example 2
local Quire = _G.Quire

local Slot = {Class = "Slot"}
Slot.__index = Slot

function Slot.new(inventory, key, slotType, itemStack)
	return setmetatable({
		SlotType = slotType,
		Inventory = inventory,
		Key = key,
		ItemStack = itemStack
		}, Slot)
end

function Slot:GetInventory()
	return self.Inventory
end

function Slot:GetKey()
	return self.Key
end

function Slot:GetItemStack()
	return self.ItemStack
end

function Slot:AddItemStack(itemStack)
	local oldItemStack = self:GetItemStack()
	self.ItemStack = itemStack
	self:GetInventory():OnSlotItemStackChanged(self:GetKey(), oldItemStack, itemStack)
end

function Slot:RemoveItemStack()
	local oldItemStack = self:GetItemStack()
	self.ItemStack = nil
	self:GetInventory():OnSlotItemStackChanged(self:GetKey(), oldItemStack, nil)
end


return Slot

aside from the extra lines each getter method makes, I think it makes the code a bit more understandable

Is it fine?
Is there a limit, or is it better to use no getter methods at all?

I see this with OOP in other programming languages
Edit: and I know in other programming languages getters are required, and that’s why I’m asking if it’s good or bad for me to be using them

3 Likes

If your getter has no extra operations other than retrieving the index from the item under a similar name, you’re probably overusing it by just having it. Java is the only OOP language I see which for some reason loves this system, but even its sister C# drops this practice whenever you have anything other than just read only fields or special logic.

The only cases I can really give a getter a reason in Lua would be read only properties, special get logic where some operation must be performed, and maybe if that’s your style. For the sake of readable I’d say don’t do the third.

7 Likes

Yeah, this makes a lot of sense

One of the main sources where I am learning how to make a inventory system properly is from minecraft code, and that’s java

I’m working myself on a module that has to do with matrices, and I stumbled across this problem where I can’t choose between having multiple getters or decrease their numbers. For example I have a :GetColumnVectors() methods, but I was not sure if I just add a :GetColumnVector() without the s, because really I can just do :GetColumnVectors()[key] manually, like in your case.

In my opinion, just stick with a :GetSlots() method, and do the :GetSlots()[key] manually.

2 Likes

It depends on what you want to get out your API’s design. If consistency is preferred, then it might be worth using getters and setters for all fields, making it easier to understand what an object’s fields actually are.

If you want to prioritize performance, then you could have a hybrid approach that uses methods for getting and setting only when necessary. Fields that need no extra behavior can just be accessed directly. I tend to use this pattern by designating lowercase fields as “private”. This is enforced only by discipline, but it makes the API easier to reason about.

You could also go all-in by utilizing newproxy() and metatables. This is the most expensive approach, but the result is actual encapsulation. This might be used if you’re writing a third-party library, and maintaining the integrity of an object’s internal state has the greatest priority.

3 Likes

I personally always use Getters and Setters in classes in Roblox because it makes refactoring a lot easier. However, this can be really cumbersome to modify data. However, in this case, the way you design your objects can make this a bit cleaner.

In this case, your question brings up an important point–this design feels clunky, but I don’t think it’s because of the Getters and Setters.

Here’s my thought process:

  1. First thought: Slots probably shouldn’t know about inventory. If they need to know about it (i.e. some consumer of slots needs the inventory), you should just be passing the whole inventory around, since clearly the consumer of the slots is really manipulating the whole inventory–not just a single slot. Furthermore, I suspect there’s very few cases where you want to isolate a single slot from the rest of the inventory in terms of refactoring.
  2. Second thought: Slots in this case are little more than data containers. The fact that the Inventory has a method called OnSlotChanged implies that the inventory isn’t benefitting at all from the encapsulation of slots, i.e. slots aren’t really decoupled from the inventory object at all. Furthermore, the fact that the inventory has a method called GetInventoryData implies that the slots arne’t even the owner of their own data! This implies to me you should simplify your design to just an inventory object.

In exploring this thought, we should consider what your inventory system might be in charge of:

  1. Source of truth for items stored in inventory
  2. Exposing changes to the inventory to the UI
  3. Serialization of data (this should be separated from the inventory object)
  4. Synchronization of data between client and server.

In this case, you probably want the server to be the source of truth. The single responsibility of the inventory in this case should be to thus be a holder of data.

I just spent like 30 minutes trying to make the OOP feel better, and it turns into a sort of messy solution. You sort of end up with a ton of objects to hold the slot contents, even though I think they’re sort of just filler.

Thinking about this problem more, you probably can get away with an immutable inventory as the source of truth. This leads to a solution like Rodux or Roact. I gave that a shot, and end up with something a bit cleaner.

I think since this problem is fundamentally about manipulating data, and not about behavior of slots (which tend to be mostly the same), then OOP is less useful in this case.

---
-- @module Inventory
-- @author Quenty

local require = require(game:GetService("ReplicatedStorage"):WaitForChild("Nevermore"))

local Rodux = require("Rodux")

local SlotReducer = require("SlotReducer")

return Rodux.combineReducers({
	food = SlotReducer(10, "food");
	potions = SlotReducer(5, "potion");
})
---
-- @module SlotReducer
-- @author Quenty

local require = require(game:GetService("ReplicatedStorage"):WaitForChild("Nevermore"))

local Rodux = require("Rodux")
local ItemReducer = require("ItemReducer")

return function(numOfSlots, slotType)
	assert(numOfSlots)
	assert(slotType)

	return function(state, action)
		if action.slotType ~= slotType then
			return state
		end

		assert(action.slotNumber)

		local newState = {}

		for i=1, numOfSlots do
			if i == slotNumber then
				newState[i] = ItemReducer(state[i] or {}, action)
			else
				newState[i] = state[i] or {}
			end
		end

		return newState
	end
end

And then to dispatch you’d do something like

rodux:dispatch({
    type = "setSlotContents";
    slotType = "food";
    slotKey = "slotKey";
    contents = { }; -- TODO: Put in item data for stack here! 
})
---
-- @module ItemReducer
-- @author Quenty

local require = require(game:GetService("ReplicatedStorage"):WaitForChild("Nevermore"))

local Rodux = require("Rodux")

return Rodux.createReducer({}, {
	setSlotContents = function(state, action)
		return {
			contents = action.contents;
		}
	end;
	clearSlotContents = function(state, action)
		return {}
	end;
})

tl;dr Maybe in this case, OOP isn’t the correct solution

11 Likes

If you can access or manipulate the data without using the methods, and the methods don’t have any other purpose then you do not need them. If the data isn’t private and you can just use Object.data to access it, then you will not need it.

1 Like

I use getters anywhere the overhead of a function call is not a performance concern. Lua is interpretted, so you always have this overhead, unlike with compiled languages where the actual function is often optimized away, so you get the encapsulation and other benefits basically for free.

Also, because Lua getters are function calls, and calling a function that doesn’t exist is a guaranteed run-time error, things like misspellings and typos in member names are easier to catch and fix than if you just accessed a member property by the wrong name, got nil, and then either have a silent failure or a bug somewhere down the line when you eventually try to use the value.

Also, as Quenty mentions, code that goes through a well defined interface is easier to refactor. It’s also easier to debug–you have one place to set a breakpoint or add debug output.

2 Likes

Not mentioned yet here: debugging!

Better debugging support is a huge benefit to consistently using getters and setters for fields. Have some really hard to track down bug where an object is getting into a particular bad state? If you’re using setter functions consistently then you can have all your setters check for that bad state, and hit a breakpoint when that condition is found, allowing you to inspect the call stack and track down the root cause. I’ve found this very effective myself – I’ve conquered many of the hardest bugs I’ve had to solve in the past by instrumenting getters and setters in this way to nail down exactly when things were getting into a wrong state.

If you had used a naked property instead you would be out of luck unless you wanted to go through the error prone process of trying to replace the access with a setter after the fact. Plus you’d be left with the inconsistency of having only that one thing use a setter afterwards.

7 Likes

I don’t see what this adds to the discussion…??

@Autterfly isn’t saying to not use OOP (at least thats all I see in there), but is saying that overusing Getters and Setters is more of a Java process than C#.

(And OOP isn’t the best way to code. OOP is good for structuring your game in a simple, readable format, but coding normally w/out OOP isn’t bad.)

2 Likes

OOP has no intrinsic relation to code reusability, programming in modules, or easy maintenance. These are all benefits of the implementation of a programming language you’re using.

If anything, OOP is the worst of the paradigms for its severe toll in performance via indirection, usual reliance on garbage collection, frequent allocation, and treating all problems as if objects inherently have ownership over some actions (i.e attack vs take damage, which to implement). But that’s a different discussion than in this thread.

Use whichever paradigm is right for the job.

4 Likes

I Will give a example:

    local Victim = {}


local module = {}

     function module.New(caract)
        local Data = {
            ["Agressor"] = nil;
            ["Time"] = 0
        }
        Victim[caract] = Data
     end

     function module.Delet(caract)
       Victim[caract] = nil
     end

     function module.Get(caract)
        return Victim[caract]["Agressor"]
     end

     function module.Set(caract,aggressor)
        Victim[caract]["Time"] = 5
        Victim[caract]["Agressor"] = aggressor
     end

     function module.DeletAgressor(caract,agressor)
        for count = 1,5 do
            Victim[caract]["Time"] = Victim[caract]["Time"] - 1
               if agressor ~= module.Get(caract) or Victim[caract]["Time"] == 5 then
                  break
               else
                  module.Set(caract,"")
               end
           wait(1)
        end
     end

     function module.attackedPlayer(caract,aggressor)
         module.Set(caract,aggressor)
         module.DeletAgressor(caract,aggressor)
     end


return module

That IS my module to check who killed the player or npc

Code reusability: on ALL my skill Scripts i can Just do Kda.attackedPlayer(caract,aggressor), so i dont need write the Full code again on ALL skills.
Easy maintenance: i wont need EDIT the code on ALL skills If I need implement something(or fix something)

About that Getters and Setters need be used only to Get/Set Variables values, to do others thinks you dont need use Gets / Sets

Maybe you should do some research, because you would find that whether OOP (or any other programming paradigm) is a good idea or not is very much an open debated question in the programming profession.

The article you post is a basic introduction to the concept, and presents an extremely simplified viewpoint.

2 Likes

Nobody is arguing that re-using code rather than copy-pasting it is a bad idea, the question is whether OOP is the best way to accomplish that. There are other approaches such as functional programming which solve the same problems in a very different way.

3 Likes

They’re not, OOP just isn’t the best for every situation. Take a look at the performance of OOP code vs DOD code (Data oriented design). There’s a few examples around on youtube, I believe. OOP would have you create new connections for each .new for each class (if there are any) while DOD doesn’t. It leads to stupid surges of performance (we’re talking being able to have 3k npcs on screen without frame drops)

In the example you gave, you are not using oop. Oop is creating a proper class, that is to say a stand-alone table containing all of the functions in it (With metatables else you will run into performance issues). You are simply using a module in the example and giving it oop-sounding function names.

Oop would have you do

local class = (module).new()
class:Get(caract)

and not

(module).new()
(module).Get(caract)

That being said, before you speak high and mighty of OOP, please get some actual experience with it, then you’ll see it’s real benefits and disadvantages. If you build an entire game in it, you will end up with an absolute mess on your hands, unless you’re some kind of savant and planned everything perfectly. You have to know when and where to use it to make proper use of it.

Please do not fall into the trap of thinking your code and coding philosophy is immune from criticism just because of previous studies. You will stall and never progress as a game dev that way. There is always room for improvement. Not even someone who has taken programming classes is immune to criticism, much like you don’t have to be a movie producer to criticize movies.

If you guys want to continue this argument, please take it to messages. This is derailing the main post.

2 Likes

That kind of unfair since Java is class-based

OOP on roblox is often not needed, and even counterproductive in times. Like half of the time people only implement it to look smart.

But I’m not gonna reply any further, I thought I might say my opinion

1 Like

Feel like someone has to say it, remember that this post is about if I’m using too much OOP getters, not about debating if OOP is good or not

1 Like

Lua is like Python in this regard. Treating it like Java is a mistake. If you are using OOP, you’re already disregarding performance enough if you’re doing any inheritance.

You do not want to call “object:getName()”, you want to write “object.Name” and if that needs to be converted to a getter function, you can do so through metatables as previously suggested. @stravant Debugging works just as well here when you can just replace the property with a getter function when you have to debug.

Quenty’s point is correct that this is definite overuse, but just in general you should prefer “property” access to getters. Write Lua code idiomatically in Lua, not in Java. Modern languages understand this (Dart, Python is old but counts), and abstract away “getters” when possible.

2 Likes

Is OOP the only paradigm you have any experience in? I think you just don’t know about common alternatives to strict OOP compliance and you’re imagining that everything that isn’t OOP is spaghetti code by default. That is not the case, there are other things besides OOP that can be better. Java doesn’t give you that choice, though.

1 Like