3
\$\begingroup\$

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?

\$\endgroup\$

closed as off-topic by Philipp, MichaelHouse Dec 9 '15 at 15:18

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Programming questions that aren't specific to game development are off-topic here, but can be asked on Stack Overflow. A good rule of thumb is to ask yourself "would a professional game developer give me a better/different/more specific answer to this question than other programmers?"" – Philipp, MichaelHouse
If this question can be reworded to fit the rules in the help center, please edit the question.

5
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Thank you. Just that I'm thinking of adding the AddChildItemData to the ItemData not an outside service. That's what I'm actually asking: is it better to keep AddChildItemData as a method of ItemData or to keep it outside of it as it's a trend now to keep all your data separate from logic/behaviour. \$\endgroup\$ – mt_ Dec 9 '15 at 12:33
  • \$\begingroup\$ @mod__ I added additional examples to the discussion, which might help you to decide for your case. \$\endgroup\$ – LukeG Dec 9 '15 at 12:50
0
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ I changed the subject to "Is it a bad practice...". I'm only looking for opinions and an open discussion not permissions... \$\endgroup\$ – mt_ Dec 9 '15 at 12:27
  • 1
    \$\begingroup\$ Then I'm sorry to say you're in the wrong place, stack exchange isn't a forum, we don't do well with open-ended questions. There are better places on the web for that. \$\endgroup\$ – Tim Dec 9 '15 at 12:36
0
\$\begingroup\$

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.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.