Is it a bad practice to add helper methods if the class is intended only for data? [closed]

Question

For example, if I have something like:

class ItemData
{
    public int cost;
    public int level;
    ...
    public List<ItemData> childItems;
}

And I'm aiming to separate data from logic, is it ok if I add methods to ItemData such as:

class ItemData
{
    ...
    public void AddChildItemData(ItemData childItem);
    ...
}

that are helper methods for setting or manipulating the data? Or should these be moved elsewhere to service classes?

Answer 1

Your design contradicts object oriented programming.

In object oriented programming (which is the core of the C# language) you combine data and logic in a class. This means a class is a construct that defines all of its properties (cost, level, children in your example) as well as what it can do. Hence you should of course combine those two parts,

---

Discussion of the matter in practice

1. Classes as pure data containers:

So much for the theory of OOP. In practice however this is not always practical to a full extent. Imagine a physics system, that has control over many objects exposed to it. Even though an object is theoretically capable of colliding with other objects it's better to leave those checks to a seperate system so it is easier to maintain, because it's centralized.

Second example: I'm doing a simple engine for a game at the moment and designed a rendering system. I created a Scene class, that holds information about all visuals (only geometry and lights). I have a second class Renderer, that operates on scenes (orders the lists for optimization and renders everything). Because the renderer can be configured in many ways all those functionalities are not included in Scene, even though they are directly tied to them. That way Scene is exclusively for holding data.

There are a lot of cases where classes are designed as pure data containers without any logic to them. However, if you have methods, that are immediately tied to your data container (Like your AddChildItemData method) there is no sense in outsourcing it. Imagine code like this:

ItemData data = new ItemData();
ItemData childData = new ItemData();
ItemDataManipulator manipulator = new ItemDataManipulator(data);
manipulator.AddChildItemData(childData);

That's not readable and looks like an awful design.¹

2. Acess considerations:

However, there's another point to be considered: If you want some parts of the logic only be accessible to some part of your code (for example the part where you load data from disc) you might lean towards creating something like a creator class to hide this kind of logic from other parts of your software (you could for example decide not to make item modification available to mods). There are many other possibilities to achieve this (mainly access modifier) you should consider.

---

Conclusion

It depends. There are cases where classes (and - even more so - structs) are designed as pure data containers. But logic, that is only useful for this container, should be included in it. As I showed above, imagine you'd have to instantiate a seperate class to add a children to one of your objects. That's usually no smart design and over-complicates things.

But, as I discussed above: The design of your software completely depends on what you want to achieve and there is not definite answer.

Listing of various object-oriented design patterns (I did not read much of it, but it looks reliable)

---

¹: Note: There are software patterns where code like this is actually common. It can be useful to hide some features in certain situations. A little example: I'm using a builder pattern for keyframe-animations in my code with various operations that could be used when creating and editing operations. When applying the animation somewhere I don't need these operations. Patterns like this can be extremely useful when you want to expose only a certain set of operations in a given context.

---

Personally I follow this paradigm:
Try to keep your code as complex as needed, but as simple as possible.

Answer 2

is it ok...

You shouldn't ask for our permission for your code. Ask whomever is responsible for code standards, if that's you then only you can truly answer the question of whether or not it's permissible.

However on the question of whether or not the use of helper functions can be suggested or follow good standards™?

In theory; Not really, a true object-oriented design should have each object be entirely self-contained and not dependant on outside helper functions.

In practice; Designing a truly object-oriented design is hard and the cost usually isn't worth it for games, as long as you're consistent a strict adherence to proper practices doesn't matter much.

Most importantly: Don't tangle yourself too far with code standards, you can write good code with bad practises and bad code with good standards.

As long as you document and be consistent! You'll be fine.

Answer 3

A core principle of object-oriented programming is encapsulation. It means that all member-variables of a class should be declared private and the only way to change them from the outside is through methods of the class. This separates the interface from the implementation.

This is especially important for complex containters like List, because when you expose these directly you allow other classes to make changes to the list which are not under the control of the class which owns the list. When adding or removing child items means that more needs to be done than just adding them to the container and you (or another programmer) forget about that, this will end in disaster.

If you need other classes to read the content of the list, it is usually a good idea to expose only the most minimal interface. When you want to provide read-only access to a List, it's usually sufficient to return an IEnumerator which allows to read the contents of the list but not change it:

class ItemData
{
    private List<ItemData> childItems;

    public void AddChildItemData(ItemData childItem) {
         // do whatever internal bookkeeping is required
         // when child items are added
         childItems.Add(childItem);
    }

    public IEnumerator<ItemData> GetChildItemData() {
         return childItems.GetEnumerator();
    }
}

Notice that this class does not expose the implementation detail that it stores its data in a List<ItemData>. That means you can easily change the way you store the information without having to change any of the code which uses it. This improves loose coupling making your code more reusable and maintainable.