29 Dec 2016

You don't need that comment!

Piotr Korzuszek
By Piotr Korzuszek Unity Development Expert

Let’s talk about programming. Since you’re a Unity developer then most probably you had to do some programming, unless you use Playmaker or something similar. As a programmer you most probably want your code to be as readable as possible, because sooner or later you will have to work on it knowing why you’ve done something in a particular way. What is your method of making your code more readable? Do you use in-code comments? If yes, you’re doing it terribly wrong.

Why comments are bad

You may now think of me as of an idealist. Thinking of a world without conflicts, religion, etc. Well, it’s nothing like that. Every source code requires more or less comments, but in most cases, when you may want to write one, most probably there’s a better solution.

Why do I state that using comments is bad?

Comments can get quickly outdated

Consider this piece of code:

void Update()
{
    // finish level after 60 seconds
    if (Time.timeSinceLevelLoad >= 60)
    {
        SceneManager.LoadScene("Score Board");
    }
}

And let’s say someone will refactor it:

public float PlayTime = 120;

void Update()
{
    // finish level after 60 seconds
    if (Time.timeSinceLevelLoad >= PlayTime)
    {
        SceneManager.LoadScene("Score Board");
    }
}

The person who did the refactoring replaced 60 seconds with PlayTime field and then made it 120. It’s a common “mistake” to leave the comment. Even if the comment would be updated, anyone who will change the PlayTime value won’t be aware of a comment in Update() function that is now lying to us.

Comments need to be read along with the code

Isn’t this a purpose of comments existence? Well it actually it is, but if there’s a comment then it means that reader needs more time to get through the code because he or she needs to analyze code and all the comments. It’s not only the matter of how much there’s to read – code that needs a comment can be understood only when reader can get understanding of a comment – so there’s as twice as much things to analyze.

Now I’d like to show you some real scenarios in which comments were used, as well as how it can be improved and why.

Use meaningful variable names

Something so obvious, yet frequently ignored. How often did you stumble upon variable names like abx2, etc.? Let’s take this code example:

// finds out distance between the player and the enemy
float a = Vector3.Distance(pl.transform.position, en.transform.position);

// if close enough
if (a < 100)
{
    // attack the player with claws or poison as secondary attack
    enemy.Attack(player, Attack.Claws, Attack.Poison);
}

First of all you should know that using short variable names won’t make your application faster. It only will make your code less readable. Always remember to use meaningful variable names. “Meaningful” does not mean long, so if you want to use a short version of variable name, you are allowed to do so, provided that it can be easily decoded (pointer -> ptr, texture -> tex). Using too long variable names can easily backfire with code that is too bloated, so you have to make compromises.

float playerToEnemyDist = Vector3.Distance(
  player.transform.position, enemy.transform.position
);

// if close enough
if (playerToEnemyDist < 100)
{
    // attack the player with claws or poison as secondary attack
    enemy.Attack(player, Attack.Claws, Attack.Poison);
}

Note that we not longer need the comment because playerToEnemyDist variable name tells us enough about what we’re doing in that line.

One more note about variable names: for loops are frequently using a counter variable. It’s perfectly legal to use “i” or “j” as a counter name since it is widely used style of naming. The same applies for “x”, “y” and “z” while we’re making operations on coordinates.

Do not use magic numbers

“Magic” numbers are numerical values that exists inside methods and it may be difficult to know why these numbers are set in a particular way. Magic numbers should be extracted into variables, fields or even class constants with meaningful names.

private static const float EnemyAttackDistance = 100;

void Update()
{
    float playerToEnemyDist = Vector3.Distance(
      player.transform.position, enemy.transform.position
    );

    // if close enough
    if (playerToEnemyDist < EnemyAttackDistance)
    {
        // attack the player with claws or poison as secondary attack
        enemy.Attack(player, Attack.Claws, Attack.Poison);
    }
}

Do not explain ifs

If you have an if statement and it may be not clear what it does and why, you may be temped to make a comment to that statement. Before you do, think of extracting your if () condition (what’s inside) into a meaningfully-named method.

private static const float EnemyAttackDistance = 100;

void Update()
{
    if (EnemyIsNearPlayer())
    {
        // attack the player with claws or poison as secondary attack
        enemy.Attack(player, Attack.Claws, Attack.Poison);
    }
}

bool EnemyIsNearPlayer()
{
    float playerToEnemyDist = Vector3.Distance(
      player.transform.position, enemy.transform.position
    );
    return playerToEnemyDist < EnemyAttackDistance;
}

Please note two things:

  1. By extracting if condition we could move distance calculations into a separate method so Update() function has much less code to read now.
  2. EnemyIsNearPlayer() function name is self-descriptive. We do not need a comment here.

Watch out for function parameters

See this code?

// attack the player with claws or poison as secondary attack
enemy.Attack(player, Attack.Claws, Attack.Poison);

How you can tell if the poison is a secondary attack without a comment? Well, you cannot. This scenario is a good example where a comment is really needed. Still, if enemy’s class is something that you can refactor, consider using a struct instead of raw parameters:

public struct AttackInfo
{
    public Attack Primary { get; set; }
    public Attack Secondary { get; set; }
}

// ...

class Enemy
{
    void Attack(Character player, AttackInfo attackInfo)
    {
        // ...
    }
}

So our code may finally look like this:

private static const float EnemyAttackDistance = 100;

void Update()
{
    if (EnemyIsNearPlayer())
    {
        enemy.Attack(player, new AttackInfo {
            Primary = Attacks.Claws, Secondary = Attacks.Poison
        });
    }
}

bool EnemyIsNearPlayer()
{
    float playerToEnemyDist = Vector3.Distance(
      player.transform.position, enemy.transform.position
    );
    return playerToEnemyDist < EnemyAttackDistance;
}

Summary

At first glance it may look like  producing more code instead of using comments for much simpler code, but trust me – this will greatly make you and your colleagues’ life much better in the long run.

Still, if you feel that a comment is really needed and there’s nothing else to do – go ahead! There are situations where you cannot do much about it. Yet remember that the information in your comments may someday become outdated, so keep it simple and safe.

Piotr Korzuszek
By Piotr Korzuszek Unity Development Expert
SIRBart

Call The Knights!

    Table of contents