Clean code

Nowadays, developers are relatively easy to start creating their own games. They can easily find tons of tutorials on youtube, vimeo, blogs (like this one ;)) or in a books. However, programming is a tricky profession – it is easy to learn but hard to master. Coding is a very flexible job – you can create things on different ways. Some of them are easy at the beginning but often the easiest way can cause some troubles when your project become more complex.

For example, on youtube, authors of courses show the simplest way to solve some problem. In tiny issues like the topic of certain tutorial it is no a problem. Later programmer will remember some bad habits from simple tutorials and he is willing to reproduce it later during development the real projects. Today we are going to talk about clean code – set of rules that allow programmers to be more efficient in their work. In this part, I will show you a "fat and ugly" Main.cs class which I find in one of projects.

Super main class

Notice this is only 1800 from 2873 lines of this class code ;)


Why meaningful names are good?

Names are really important in programming. They are everywhere in software. We name variables, methods, arguments and classes. We continuously set names. Because we use it so much, we should do it well.


Use clear intention-revealing names

Good name of a variables, methods or classes should reveal our intentions – it should answer all of important questions. Good names should tell you why it exists, what it does and how to use it. If name of variable require extra comment – it isn’t good name.
BadNames

Simple example from *Main.cs* class “How do not name variables”

The name hvdestroyCount did not give us a lot of informations. We better should name it something like that:

int horizontalAndVerticalActiveRoutinesInFirstDestroyCounter;  
int horizontalAndVerticalActiveRoutinesAfterFirstDestroyCounter;  

You can notice that those names are pretty long. But please not use it like an argument for use short and not clear names (like hvdestroyCounter). You have available an amazing tool called IntelliSense in Visual Studio so you can no longer complain about long (and readable!) names.

Look at example function (from Main.cs) which use one of above variables. Is it clear for you what it does?
ExampleOfFunction

How will it write more legible? For example:

void IncreaseHorizontalActiveRoutinesCounter(int horizontalArrayIndex)  
{
       if (horizontalArrayIndex > 1 &&  horizontalArrayIndex  < 9)
       {
           horizontalAndVerticalActiveRoutinesAfterFirstDestroyCounter++;
       }
}

It looks more clearly now right? I omit meaning of this strange double if but the next question is what is hiding behind the values 2 and 9?

Do not use “bare” numbers

In case of using only numbers in your code - maybe now you understand what does 1 or 38 mean. But in the future you or other members of your team will have a problem with understanding of your present intentions. It is wise to use constant value to describe some number.

const int MIN_SPOT_TO_DESTROY_INDEX = 1;  
const int MAX_SPOT_TO_DESTROY_INDEX = 9;

 /...
 if (horizontalArrayIndex > MIN_SPOT_TO_DESTROY_INDEX &&  horizontalArrayIndex < MIN_SPOT_TO_DESTROY_INDEX)
 /...

Trust me it is a lot of more understandable to other coders (and for you too).


Classes and methods naming

Classes and objects should have noun or noun phrase names like Enemy, Player and DataConverter. A class name should not be a verb – verb is reserved for methods. They should have verb or verb phrase names like AddExperience, ReceiveDamage or SearchEnemy. What about first letter of name - it should be capital letter or lowercase? In University of Games we use the Unity Engine notation. So we start classes and methods from capital letter and variables from lowercase.


Clean functions

Functions are the first line of organization in our code. So it really good for you if you write them well. First of all:

Good function is small function

In “Clean Code” book, Robert C. Martin says that there are only two rules of functions:

  1. They should be small
  2. They should be smaller than that.

In Unity we have important method called Update(). It is good idea to split Update code in separate functions. For example Update() in Main.cs class has 166 lines. 166 lines of spaghetti code. In this case, same idea of superclass Main was bad idea. If we had many complex operations during Update() you should group them in small functions with good names. When we look at this code we know what author meant by that. Maybe we try refactor one of pretty methods from Main.cs.

void DestroyIfToLess(){ // unmarking spots if is marked less then tree spots  
        int howMuchIsMarked=SpotCount.numberOfMarkedSpots;
        spotsScripts = originSpot.GetComponentsInChildren<Spot> ();

        if ( howMuchIsMarked < 3) {
            foreach (var item in spotsScripts) {
                item.ID=0;
                item.pressed = false;
                item.over = false;
                SpotCount.numberOfMarkedSpots--;
                item.SpritePressed (false);item.ID=0;
            }
        }

        SpotCount.numberOfMarkedSpots = 0;
        images=originConnectingLines.GetComponentsInChildren<Image>();

        foreach(var item in images){
            item.gameObject.SetActive(false);
            Destroy(item.gameObject);
        }
    }

How could you know, how to divide this block into smaller functions? It is simple:

One method do one thing

Code above doing lots more than one thing. It’s creating references, resetting states of “item", turning off GameObject and finally destroying them.

Functions should do one thing. They should do it well. They should do it only.

So,let’s try to refator this little monster. What we can change before? We starts from divide them into smaller functions? First of all change names and remove useless references and repetitions. Secondly I decided to devide DevideIfToLess() to 4 smaller functions: CheckIfMinimumCountOfSpotsMarked, ResetSpotsState, ResetSpot (in Spot.cs now) and DestroyConnectionsImages.

Finally this method after small refactor should be like this:

   void UnmarkSpots()
    { 
        if (CheckIfMinimumCountOfSpotsMarked() == false)
        {
            ResetSpotsState();
        }

        SpotCount.ResetMarkedCount();
        DestroyConnectionsImages();
    }

    bool CheckIfMinimumCountOfSpotsMarked()
    {
        if(SpotCount.numberOfMarkedSpots < MIN_SPOTS_COUNT_TO_VALID_MARK)
        {
            return false;
        }
        return true;
    }

    void ResetSpotsState()
    {
        Spot[] spots = originSpot.GetComponentsInChildren<Spot>();
        foreach (Spot spot in spots)
        {
            spot.ResetState();
        }
    }

    void DestroyConnectionsImages()
    {
        Image[] connectionsImages = originConnectingLines.GetComponentsInChildren<Image>();
        foreach (Image image in images)
        {
            Destroy(image.gameObject);
        }
    }

//In Spot.cs
void ResetState()  
    {
        ID = 0;
        pressed = false;
        over = false;
        SpritePressed(false);
    }

It is more clearly and easier to understand right?


Conclusion

To choose good names in programming structures are required good descriptive skills. With some training you should simply learn it. Often programmers afraid of renaming things for fear that it will make more problems than it's worth. We believe that change names to more legible is always good idea. Follow some of above rules and check if they improve the readability of your code. The important lesson is start with idea of small functions. Smaller methods = smaller problem later ;)

I hope that after lecture of this article, you will think that idea of one super class Main is really terrible idea. Clean code is a really wide topic – today we have talked only a small part of whole topic. Stay tuned, in future I will tell you more about another aspects of clean code :-)

I hope it’s useful for you. If you have any comments or questions, please write them here!

Mateusz Olszewski - Lead Programmer



Show Comments