Search code examples
c#unity-game-engineoopcode-cleanupsingle-responsibility-principle

Is breaking my class into multiple classes, the right thing to do?


I'm working with Unity and C# and I have a canvas that is about a competition with several milestones. So I have to show a leaderboard and a slider for the player's progress in the competition and the rewards they'v got since now and also some other stuff like the title.

I have a class named CompetitionCanvasManager to handle all these things but actually, they are not related to each other in code and all of them just use one single variable in common to set the proper UI. If I break this class into 3 different classes such as LeaderboardManager and RewardsManager and SliderManager and use my current class to just initialize those 3 classes and set the title, is it a cleaner code than what I have right now? Every class would have only one or two methods and probably something like 50 lines of code but I'm not sure doing this is actually better than having one less-than-200-lines-of-code class to do them all.

I'm aware of Single Responsibility Principle but in both cases, managing a canvas or managing every part of it looks like a Single Responsibility and I need help to understand what is the right way to do.


Solution

  • Single Responsibility Principle (SRP) is one of the many software development principles and practices that can be interpreted multiple ways. It's an organizational approach, and any time organization will involve more than a single attribute it can be sliced different ways and in different orders.

    A real world example

    You have a 10x10 warehouse rack full of 10 widgets and each comes in one of the same 10 colors. Do you put all widgets of the same color on the same row, and the columns are by type? Or type on the same row and columns are color? Both are valid and clearly organized, but you have to step back and look at how they will be used.

    When people order multiple widgets do they usually order multiple colors of the same widget, or multiple widgets in the same color (for fitting a color scheme)? Do your order pickers have to return to the ground before moving to the next column, or can they move across a whole row without going back down? You'll want to organize it based on how efficient your pickers are going to be able to move through the majority of their orders.

    Back to development

    You're in a good mindset to be thinking about this structure now while your class is small. Like the warehouse example, you currently have two attributes to organize by. What it works with: canvas. And what it does: Leaderboard/Rewards/Slider. Some people I have worked with in the past would argue that since they're all canvas based classes they can be kept together. And they might be right. But they also serve very different purposes.

    It sounds like you've done a good job of creating very separate classes already even though they're lumped together right now. They're not dependent on one another. They probably shouldn't be, even as they grow over the course of development. Separating them out now will help you in maintaining that separation as you go. If they're all inside the same class then the temptation to use shared variables increases. When you start out with them separate it forces you to use better methods for communicating state between them such as callbacks.

    What's the true payoff for this work?

    You view this as an SRP issue, but the consequences reach very far and into other principles. A set of classes that are kept separate and have proper ways of interfacing and communicating with each other are easier to test. Unit Testing is a very useful tool for both initial development (Test Driven Development) and in helping prevent bugs from appearing (Regression Testing) as you make changes. Unit Testing depends heavily on Dependency Injection to properly isolate the code being tested.

    A mega class with many different methods that share common variables is harder to test because changes to those variables caused by one method may affect how another behaves. If you're not already aware of the dangers of "global scope" variables you should read up on it. When classes get too large, because they're trying to do a bunch of loosely related things all in one place, the class level variables that are used throughout become more and more likely to be as dangerous as global scope variables.

    Final thoughts

    I'm a bit of a structure snob in my work, and I definitely would be splitting those classes up. It may seem excessive right now, but future you will likely thank you for the minimal effort you put in at this stage. And lastly, as an example of when splitting classes up becomes more obvious, think about data access classes for a large system.

    We have a project at work that has a lot of tables and views. We could have put all the data access methods into one class and said, "its single responsibility is to access the database." And that class would be thousands of lines long. Instead, we have them split out by the entities they work with. As a result, we had to create a manager class that makes it easier to wrangle all those classes in the upper levels. And as a result of having the manager class, we now have a place where we can handle transaction scoping for all our data access.

    The manager class is what actually handles the commits and rollbacks. So no single data access class has to ever be aware of what other data access class is calling it, or how they behave. They simply do their reads, updates, etc and we know that the manager will handle the rest. Their actual single responsibility is to interact with the database for every action that happens on their respective table/view.