OOP Best Practices

Development Programming & Scripting C++
, ,
1

I’m always trying to keep things simple and logical when writing and organizing my code. However, when it comes to certain Getter & Setter methods, I question how far it should go. For example:

I have a Player Character that has an Attributes and Inventory component attached. Some objects, aside from the Player Character, may need to interact with these components, such as my UI. I understand that you want to contain functionality as much as possible so generally speaking you don’t want objects to set properties that don’t belong to them. In this case, the UI belongs to the Player and the Inventory Component belongs to the player. Therefore, I would have the UI run a method on Player and Player would then run a method on the Inventory component to set it’s properties.

With that said, how far should this be taken? If my UI just needs to know a property value from my Attributes Component, I understandably need to have the UI communicate with the Player and the Player needs to return the value to the UI from the Attributes Component. But, should I have a “GetProperty()” method on my player that calls a “GetProperty()” method on my attribute in order for my UI to get it’s value or is it better practice to simply have a “GetProperty() {return Attribute->Property;}” method on the Player that the UI calls? The chain of hierarchical getter functions could extend beyond rational use in a large complex game.

Again, I understand that setting properties should always be done by the owning object when at all possible so having the extra Setter method is understandable. I was going to ask this as a question as I’m truly interested on everyone’s opinion but I have a feeling this may be slightly contentious or at least a varying degree of opinions so I left it as a Discussion.

Thanks

2

In developer forums this is a quite common question.
To answer it short : It depends.

My opinion:
I try to use getters and setters all the time because you often have further requirements in later stages of development, which require things like raising events when values are changed and so on. With methods you are much more flexible in these cases and don’t have to rewrite huge parts of your code.

The more functionality you have, the more your code should be structured.

So I use ActorComponents extensively and add them to the Actors. I many cases the Actors are only dumb objects for me and are only defined by their ActorComponents. Then I use interfaces to call the ActorComponents functionality from outside, if the Actor implements it.

3

Why? Your UI implementation already assumes that the Inventory Component is already present, why not access it directly? Why use the player as the middleman?

If both the player and the Inventory component need updating, you could also do it via delegates. Instead of the UI calling the player and the InventoryComponent, the InventoryComponent and the Player would listen to the UI.

With that said, how far should this be taken? If my UI just needs to know a property value from my Attributes Component, I understandably need to have the UI communicate with the Player and the Player needs to return the value to the UI from the Attributes Component.

Why? If your UI implementation already assumes there is an Attributes Component, again, all you’d really need to do with the player is to provide a reference to that component. Why use the middleman since your implementation already assumes that the UI and the Attributes component go hand in hand?

But, should I have a “GetProperty()” method on my player that calls a “GetProperty()” method on my attribute in order for my UI to get it’s value or is it better practice to simply have a “GetProperty() {return Attribute->Property;}” method on the Player that the UI calls? The chain of hierarchical getter functions could extend beyond rational use in a large complex game.

If you are just going to call the property directly like that I would not use getter or a setter. You can appropriately mark variables as private or public.

I would use getters/setters when I’m trying to safeguard (i.e. nullpointer checks), modify the value before getting it (i.e. normalizing instead of giving a raw value).

Even inside Unreal Engine, if you look at CharacterMovementComponent, stuff like GravityScale are directly accessible as public variables with no getters or setters. However, the movement component itself in the character class has a getter, presumably because this prevents the user from changing the movement component’s pointer to point to something else.

Again, I understand that setting properties should always be done by the owning object when at all possible so having the extra Setter method is understandable. I was going to ask this as a question as I’m truly interested on everyone’s opinion but I have a feeling this may be slightly contentious or at least a varying degree of opinions so I left it as a Discussion.

IMHO rather than thinking too hard about going with a strict rule of getters / setters, if you feel like a variable is being accessed from too many different places, then take a good look at the code and think if there’s some smarter way to organize it.

4

The answer to “Why” in both these instances is because one of the core principles of OOP is encapsulation. Setting all your variables and methods to Public just for the sake of accessing it by whoever you pass a reference to creates nightmare scenario’s in large scale projects. In my examples, I use the UI as a single example for needing to get and/or set variables on the components that belong to the player but extend this. I’m being attacked by 10 trolls, 3 goblins and 2 sorcerers, now I have to give different classes direct access to my players components as well as re-write methods in each class to access the information it needs. Now, what if I need to change some of the code these classes are accessing? I then have to find all classes that have access to these components and refactor each one individually instead of calling the primary method on the class that actually owns the component these other classes are accessing.

Sure, Interfaces and delegates can be used in some instances. When multiple unrelated classes need access to the same information, Interfaces makes sense because you don’t have to artificially force a class relationship. However, to send a message to the component to get the required information still requires access to the object it is trying to send a message too which means you are still passing the same pointer around to many objects allowing them all access to a component/class that doesn’t belong to them.

Here’s a real world type example that can be extended into gameplay: Assume you are at a merchant to buy some product. You as the Player have a component Wallet that tracks all your finances. When you buy the items, do you hand the merchant/cashier your wallet and say “Here, figure out how to pay and I’ll account for it later” or do you personally go into your wallet and decide how the merchant should be paid?

It may seem like I’m over thinking this, and I very well could be, but from what I understand about OOP is that giving direct access to objects/classes that don’t own the object/class it is trying to access is bad practice and breaks a core principle of OOP.

Many properties/variable have to be accessed by many different classes or objects. There are many things in the world that will affect the Health of a player so to say you need to refactor or rethink a smarter way with how you code beyond getter/setter functions may be an exercise in futility when a simple “Get/Set this information for me from this object you own” is a simple and straightforward approach. If I change how that information needs to be disseminated, I only have to refactor a single method, on the player, and all the other objects still just call GetInfo() on the player.

Again, interfaces and delegates are a valid way to go when approaching these things, but you still need direct access, at least in the case of Interfaces. Event Dispatcher type delegates are different in this regard. However, the Player Pawn is accessible to virtually any object or class in the engine because the Player Pawn is the thing which everything else in your world relies on to exist. So, rather than give access to a component that the Player has and owns, interfaces or delegates should still be called directly on the player to either Get or Set the properties on it’s own components. This same logic, I believe, holds true to AI objects that have their own implementation of components.

Thank you for your response and suggestions! I really do appreciate the discussion and am interested in other peoples approaches.

5

Thanks and I agree in the use of getter/setter style but perhaps I didn’t quite ask this properly. Since you are a proponent of components, then you should understand what I mean. Let me explain it better.

Since the Player owns these components, if I want to Get information from a component (either from the Player or an unrelated class/object) is it acceptable to make the Getter function on the player something like:

public:
GetProperty()
{
return Attributes->SomeProperty;
}

OR is it better practice to write:

public:
GetProperty()
{
return Attributes->GetProperty();
}

Where Attributes is a reference to my Attributes component. The method is made public so any unrelated class that needs this information can call it as long as they have access to the player object.

In the second approach, I am calling an extra method on the attributes component also called GetProperty() to retrieve the information for me. The second approach makes sense in terms of encapsulation (a core principle of OOP) because I can keep the property in question marked as Private or Protected and only return it if explicitly asked by the component.

However, it seems a little excessive. If I only ever give access to my Player’s components via the players own Getter/Setter functions, then by making my reference to the components Private or Protected, should theoretically achieve the same goal - encapsulating the component - as long as I ensure I never give access to the Player’s reference to the components.

Hope my questions is a little more clear. Thank you for your reply!!

6

The answer to “Why” in both these instances is because one of the core principles of OOP is encapsulation.

Yes, to avoid creating god-classes and monolithic code. For example, having an InventoryComponent would not only allow the player to have an inventory, but an inventory could be created for the enemies, who could then use the items within it with their own methods.

Setting all your variables and methods to Public just for the sake of accessing it by whoever you pass a reference to creates nightmare scenario’s in large scale projects.

There is a reason why the default access specifier in a C++ class is private. Nothing is public unless you set it to be. Rust goes even further and variables are constants by default.

In my examples, I use the UI as a single example for needing to get and/or set variables on the components that belong to the player but extend this.

But if you later want to use the UI and Inventory Component on something that is not your current player class? What do you do then? Make a new base class with the common components and inherit both of the classes from that? What if you need a third class, which has different common components?

I’m being attacked by 10 trolls, 3 goblins and 2 sorcerers, now I have to give different classes direct access to my players components as well as re-write methods in each class to access the information it needs.

Finding the right level of abstraction where you can find the most common denominator is where the magic happens. Your UI doing operations on something that has an Inventory, so it likely implements the InventoryComponent. Your enemies are attacking something that can be attacked.

Now, what if I need to change some of the code these classes are accessing? I then have to find all classes that have access to these components and refactor each one individually instead of calling the primary method on the class that actually owns the component these other classes are accessing.

I’d still argue that if you insist on a getter and a setter, instead of placing a function in the player class, you place that function in the Inventory Component class.

Sure, Interfaces and delegates can be used in some instances. When multiple unrelated classes need access to the same information, Interfaces makes sense because you don’t have to artificially force a class relationship. However, to send a message to the component to get the required information still requires access to the object it is trying to send a message too which means you are still passing the same pointer around to many objects allowing them all access to a component/class that doesn’t belong to them.

In such a situation the InventoryComponent would function as more or less a “code tab” for the character class, with a direct dependency. The UI would depend on the Player who then depends on having the InventoryComponent.

While it’s not strictly always bad - the CharacterMovementComponent is an example of a component that makes plenty of assumptions about its owner - I think such dependencies should be avoided if possible.

Here’s a real world type example that can be extended into gameplay: Assume you are at a merchant to buy some product. You as the Player have a component Wallet that tracks all your finances. When you buy the items, do you hand the merchant/cashier your wallet and say “Here, figure out how to pay and I’ll account for it later” or do you personally go into your wallet and decide how the merchant should be paid?

I’m not really sure how this pertains to the UI example.

How I’d do it, though: Player’s WalletComponent would have a “PurchaseItem” function, which would have ItemPrice as an input variable. The PurchaseItem function would likely be called by the MerchantUI. If all the necessary checks are valid, the money is substracted and item added to the inventory.

So, in this case, the merchant would “ask someone with a wallet” if they have enough money and then make a purchase. If whatever they are interacting with doesn’t have a wallet, nothing happens.

It may seem like I’m over thinking this, and I very well could be, but from what I understand about OOP is that giving direct access to objects/classes that don’t own the object/class it is trying to access is bad practice and breaks a core principle of OOP.

Even in Unreal Engine’s source code, there is no “GetGravityScale” or “SetGravitySCale” within the character class. GravityScale is a public variable in the CharacterMovementComponent. You have a getter for CharacterMovementComponent, but not for the CharacterMovementComponent’s variables.

Yet for the in-built damage system, there is a generic “Apply Damage” method.

7

I know we are in the C++ section of the forum, but I’d like to mention Blueprints anyway in case you’'ll end up using those later.

Taking the UI example, the WidgetBlueprint would be using the PlayerCharacter Blueprint as in cast and then call the GetProperty() function on it. While the PlayerCharacter has the InventoryComponent and implements the GetProperty() function. This basically introduces a dependency from the WidgetBlueprint not only to the PlayerCharacter but also to the InventoryComponent and any other dependencies that PlayerCharacter may have. This can be as bad as ending up referencing all your assets.
If we were going to slightly change the WidgetBlueprint towards what @JoSf suggested we wouldn’t need to cast to PlayerCharacter at all. All we need is some Actor that may or may not have an InventoryComponent or something implementing a specific interface that has a getter providing an InventoryComponent. Therefore our WidgetBlueprint would only depend on what it actually needs to do its job, the InventoryComponent.

If you can’t or don’t want to put PlayerCharacter specific functionality into the InventoryComponent directly, you could also derive a PlayerCharacterInventoryComponent from it. Although one could probably argue that these functions could be beneficial to anything else that may have an InventoryComponent.

Edit: also worth mentioning C++ Core Guidelines

8

Interfaces and ActorComponents are the correct way of doing it. This will reduce dramatically the amount of work you will need when expanding your system. Based on what you want to achieve, if you take examples of real world use scenarios (games) you will understand why this is by far the correct way.

Lets take a real game here (WoW as it was played in 2006) and the operations that can be performed with inventory, cash and UI:

  • you pick items on the floor and they go to inventory
  • you can trade items with other players - the UI here is important to avoid scamming
  • you can send and receive items by game mail - it has a different UI
  • you can trade items with merchants, selling or buying them, which will give or take your gold, again another different UI
  • you can carry a certain number of bags (lets say eight) and each bag can have a different size (number of slots, 1 slot per item) and the sum of their size will be the total amount of items you can carry (does not include gold which is separate and has an infinite size)
  • you can have pets and these pets also can have a certain number of bags and they can carry items aswel, but they can’t trade with other players and they can’t carry gold

Just for the amount of functionality described above tells how much one class would need to know about other classes to work properly and how a change or expansion of the system will produce side-effects and rework.

Now, if you pick that description and create interfaces that describe those needed operations, you can group the functionality and implement them inside components, with this all you need to know is the existence of such components attached and you know which functionality they implement as interfaces.

Note that on purpose I didn’t include the following feature:

  • you can loot defeated enemies and grad their inventory items

And it will be a piece of cake implement that with the concepts @HAF-Blade and @JoSf proposed.

9

Yes, you wouldn’t need the PlayerCharacter at all, though early into a project I wouldn’t mind “dumb” casting to get the reference and then if you find yourself using the UI for multiple classes, you can trivially change the method of getting the reference.

10

The ultimate goal with any of my code but especially components is portability. My intent is to allow multiple classes to implement this component and it work seamlessly.

In this example I’ve used it with the UI (but I hate that it’s been limited to this solo example when it comes to my question of encapsulation), the UI doesn’t have direct access to the inventory and honestly in my opinion it shouldn’t, it doesn’t belong to the UI.

However, if what I’m understanding you are saying is that the UI, in this example, could send an interface message to retrieve the info it needs directly? I like the sound of the approach but I’m having difficulty wrapping my head around how it would work. As I mentioned, the UI, while some pieces need info from the component, it doesn’t know the component exists and in order to send a message to something that inherits an interface, you still need access to that particular object. The way I see it is either I could give a reference to the components it needs information from and call all the setter/getter directly on that reference when needed (this seems wrong to me) or when I need something from the components, I send a message to the “owning player” of the widget, in this case the player, asking for a temporary reference to the component so it could then execute the getter/setter methods it needs from within the UI? All the second option does is prevent a single cast to the player character itself which the UI could store permanently. How would this be any different than just calling the player object to execute the getter/setter on the components?

I do agree and understand that for maximum portability I should not call functions on a player when the info I need is from something else. In this case, the UI will only ever belong to the player so getting a reference such as “Get Owning Player Pawn” is super easy and you don’t need to cast to run an interface call on it. For other instances, you can use like an “Instigator” parameter and use that reference to make the interface call.

Sorry, I know I’m making this super confusing. We’ve deviated far from what I originally asked but I see the merits in everyone’s responses.

Thanks for the knowledge

11

I agree and am coming to light on the use of Interfaces in this regard but as you mentioned here, all you need is to know is the existence of such components. In some cases, getting access, at least easy access, to the components is not always easy or straightforward. You need to have the object that owns the component pass a reference on to the caller of the interface in order for the caller to manipulate the methods and properties inside the component. Correct?

@JoSf
I appreciate your response and the time you took to help me understand this. Some of your replies to my quotes seemingly prove what I said in the quotes :joy: so I am going to spend a little more time re-reading what you wrote a few times to try and grasp it all.

Thanks again

13

You can pass the reference of InventoryComponent to the UI when the player spawns the UI. The UI only needs a reference to the InventoryComponent, not to the player.

Why would it be a problem for the UI to have a reference and directly operate on the inventory, if the UI was spawned by the player who “owns” the inventory in the first place?

How would this be any different than just calling the player object to execute the getter/setter on the components?

Now any player class that implements the UI and the InventoryComponent would also have to implement those functions.

14

True unless you create a base class of player which is the logical thing to do with any class you are going to implement and slightly tweak within the game. Base class shares common functionality while implementing their own subtleties but I get your point.

I use to do this, especially when just using pure blueprints. At some point I heard that this isn’t a great thing to do but I don’t recall why. While I’m not new to coding, I am new to C++ and figured as I’m going through this journey it would be a good idea to try and follow best practices. I don’t work as a team, I’m a solo dev/hobbyist. Working in just blueprints, following proper OOP practices is pretty limiting in nature so it’s easy to just do whatever. Physically writing the code can become very confusing and entangled quickly if you’re not careful. It’s easy to see the importance of following a good standard such as OOP.

If the common consensus is that passing around pointers so that whoever needs them can store and access them directly is common or good practice than by all mean I won’t lose any sleep over it! It will actually make life so much easier. But as I said, it’s easier to form good habits while learning than it is to break bad habits when you’re an “expert” so I’m trying to get off on the right footing.

Again, I really do appreciate the time you have taken to have this discussion with me! And surely it will help others in the future as well.

15

Yup, but heavily relying on inheritance over composition is just a roundabout way of creating god classes. The particular example being discussed does not make it obvious, but even in this case we can avoid creating dependencies where there need to be none.

Imagine a situation where you have an enemy, who does have an inventory. Now, give the player a possession ability, where they can briefly possess the enemy and control them. And, to use the enemy’s Inventory, you would use your UI implementation.

How much does the player class and the enemy class have in common? Probably not enough to justify a common parent class, at least unless you start digging very deep into the hierarchy. If your UI only relies on the component to exist, using it would be trivial.

Edit: I think more realistic example might be using the UI and Inventory Component in another project.