This series aims to teach the wisdoms of Code Zen:
Write less code that’s faster, more readable, easier to modify, and has fewer bugs.
It is very sad to see so many beginner tutorials that don’t even attempt to teach programming best practices even though it would actually make the code easier to follow and learn!
I like to fill in this space because writing better code is not hard!
The DRY principle
“Don’t repeat yourself” is the easiest principle to follow and the one that has the greatest impact on code quality.
Simply keep in mind that whenever you see the same sequence of statements repeated twice or more, you are repeating yourself.
In its simplest form, being repetitive often starts with this:
if (go.GetComponent<Something>() != null)
go.GetComponent<Something>().CallMethod();It looks harmless enough and it is widespread. From there, it grows.
Here’s what actually happens when you run this code:
// compare each type in the list of components, return match
var something = go.GetComponent<Something>();
if (something != null)
{
// compare each type in ... well, see above
var somethingAgain = go.GetComponent<Something>();
somethingAgain.CallMethod();
}As you can see, the first call to GetComponent looks up the matching component type in the list of components and returns it.
Then, if the component is not null, it does the same thing again!
(Let’s ignore any possible type caching for a moment – we don’t really know whether or when that happens.)
Then we call a method on the component that we already had, but was discarded.
And this is just the tip of the iceberg. I said it grows from there, and it often does. Possibly like so:
if (go.GetComponent<Something>() != null && go.GetComponent<Something>().GetComponent<OtherThing>() != null || go.GetComponents<Something>().GetComponent<OtherThing>().GetComponent<ThirdThing>() != Null)
go.GetComponent<Something>().GetComponent<OtherThing>().GetComponent<ThirdThing>().CallMethod();Not only is this inefficient, it’s also highly unreadable. And not just because of the line-wrapping in this post.
This kind of repetitive code is not just hard to change, it’s very prone to bugs (and hiding them) because you can’t see the forest for the trees. Our minds only skim over such wall of text, pattern matching confirms that everything “seems alright” …
=> Did you spot all three mistakes in the code above? 😉
If not, try finding those and take note how much effort and time it takes you to spot them!
Sure, in reality you have the compiler throwing errors at you. But you’d still need to take a close look to figure out what and where in this very long line the issue is hiding.
And then you need to fix them one by one because the compiler often can’t tell the other issues until the previous one is fixed.
Just remember that every time you need a method’s return value again, you assign it to a (local) variable to re-use it as needed:
var something = go.GetComponent<Something>();
if (something != null)
something.CallMethod();Excursion: TryGetComponent
With a little knowledge about the API you can actually condense the code back to two lines again if you need the ‘something’ variable only inside the scope of the if block:
if (go.TryGetComponent<Something>(out var something))
something.CallMethod();The last style would also help to improve the more convoluted code example to some extend:
if (go.TryGetComponent<Something>(out var something) && something.TryGetComponent<OtherThing>(out var other) && other.TryGetComponent<ThirdThing>(out var third))
third.CallMethod();Admittedly, this isn’t perfect either, but it’s also code one shouldn’t write anyway – a series of chained GetComponent calls within the same object is pointless and can simply be replaced with:
if (go.TryGetComponent<ThirdThing>(out var third))
third.CallMethod();Which reminds me: I should write an article about method chaining and why this makes your code harder to debug.
Example: “Wet” code hides bugs!
Here’s an instructive real-world example. It’s taken from this informative thread where a user asks How Could I Simplify My Code and Make It More Efficient?
This snippet shows how repeating yourself (among other things) can cause bugs that are difficult, if not impossible, to spot.
Here’s the code, slightly adapted for readability:
var pos = transform.position;
var origin = new Vector2(pos.x, pos.y - 0.7f);
var hit = Physics2D.Raycast(origin, Vector2.down, 0.3f);
if (hit.collider != null && isGrounded)
{
int choice = Random.Range(1, 3);
if (choice == 1 && !isOnTopOfAnotherEnemy)
{
StopCoroutine("AIMove_Event");
AIMoveCoroutine_isRunning = false;
Jump();
}
if (choice == 2 || isOnTopOfAnotherEnemy)
{
StopCoroutine("AIMove_Event");
AIMoveCoroutine_isRunning = false;
chosenMoveName = "MoveLeft";
}
}
else if (hit.collider != null && !isGrounded)
{
chosenMoveName = "MoveLeft";
}
else if (hit.collider != null && hit.collider.CompareTag("Enemy"))
{
int choice = Random.Range(1, 3);
if (choice == 1)
{
StopCoroutine("AIMove_Event");
AIMoveCoroutine_isRunning = false;
Jump();
}
if (choice == 2)
{
StopCoroutine("AIMove_Event");
AIMoveCoroutine_isRunning = false;
chosenMoveName = "MoveLeft";
}
}
else
{
transform.Translate(Vector2.right * speed * Time.deltaTime);
}
Well, there’s a lot going on here, so let me zoom in on the issue by removing the blocks of conditional code.
I’m sure you haven’t noticed the glaring issue. What about now?
if (hit.collider != null && isGrounded)
{
// ...
}
else if (hit.collider != null && !isGrounded)
{
// ...
}
else if (hit.collider != null && hit.collider.CompareTag("Enemy"))
{
// ...
}
else
{
// ...
}It should be noticable upon close scrutiny but the collider null test occupies both horizontal and mental space that can easily throw you off guard. Your mind still has to process it every time.
Let’s apply the DRY principle and remove the needless, repetitive null test. This lets you focus on just the conditions, which also naturally align better to reveal the glaring mistake:
if (hit.collider != null)
{
if (isGrounded)
{
// ...
}
else if (!isGrounded)
{
// ...
}
else if (hit.collider.CompareTag("Enemy"))
{
// ...
}
}
else
{
// ...
}When was the last time you had a bool variable that was neither true nor false? 😮
Right!
The second “else if” will never, ever execute!
Whatever the code is doing when colliding with an “Enemy” – it’s just not happening!
With that, I’ll leave you until the next article in this series. 🙂
Leave a Reply