Loading...
Refactoring Techniques - Extract Function
03-16-2025
Introduction
Programs can be quite complex; involving multiple steps, actions, and functions. Sometimes, we get carried away; writing functions that grow into massive hundred-line behemoths containing multiple different actions (I've seen some thousand-line behemoths in industry 🥲). If you've ever been cursed with needing to update or add new features to those behemoths, you know the struggle of reading and tracing them, line-by-line, just to understand:
- What the function is doing
- How each part of the function interacts with other systems
This can lead to some seemingly pretty menial tasks taking days, if not weeks, to complete. We'd like to avoid this technical debt by ensuring all our functions are neatly written, encapsulated, and properly named.
Design: "Hey, can you make it so that the skeleton uses a gun instead of a crossbow?"
A few hours later...
You: "Oh God, the code involving equipping weapons was 806 lines and for some reason after updating it, the skeleton now has a missing hand 💀 "
Problem
Let's say we're trying to create a "Special Attack" function for a fighting game (let's use Unity for the example). The steps for the special attack, as requested from your amazing design team, are as follows:
- If the cooldown is not yet reached, don't do anything
- Play a "special" attack animation
- Get the first enemy in front of the player
- Deal damage to that enemy
- Teleport forward towards the enemy (but don't go past them).
- Reset the cooldown timer
Here's what that script might look like as a first draft:
// PlayerCharacterController.cs
public const float SPECIAL_ATTACK_RANGE = 30.0f;
public const float SPECIAL_ATTACK_BASE_DAMAGE = 5.0f;
public const float SPECIAL_ATTACK_TELEPORT_DISTANCE = 5.0f;
public const float SPECIAL_ATTACK_COOLDOWN_SECONDS = 1.0f;
private float _specialCooldownTimer = 0.0f;
public void PerformSpecialAttack()
{
// check if the cooldown has been reached yet
if (_specialCooldownTimer <= 0.0f)
{
PlayerAnimationController controller = GetComponent<PlayerAnimationController>();
controller.PlayAttackAnimation("special");
RaycastHit[] hits = Physics.RaycastAll(transform.position, transform.forward, SPECIAL_ATTACK_RANGE);
EnemyCharacterController enemy = null;
for (int i = 0; i < hits.Length; i++)
{
enemy = hits[i].transform.GetComponent<EnemyCharacterController>();
if (enemy != null)
{
// deal damage
enemy.TakeDamage(SPECIAL_ATTACK_BASE_DAMAGE);
enemy.PlayHurtAnimation();
break;
}
}
if (enemy != null)
{
transform.position = enemy.transform.position - transform.forward * SPECIAL_ATTACK_TELEPORT_DISTANCE;
}
_specialCooldownTimer = SPECIAL_ATTACK_COOLDOWN_SECONDS;
}
}
This code can be quite a headache to debug or update. Now, imagine the design team changes their mind and wants the player to teleport past the enemy instead. Suddenly, you're stuck digging through this massive function just to find where the teleportation happens.
We're doing quite a few things in this function. As a general rule of thumb, if you feel the need put comments to describe each part of what your function does, you should consider refactoring and breaking it up into smaller pieces.
This function is trying to do too many things at once: checking cooldowns, handling player animations, detecting enemies, damaging enemies, handling enemy animations, teleporting to enemies, and resetting the cooldown timer. To improve clarity and maintainability, we'll use the extract function refactoring technique. This means breaking down the function into smaller, well-named methods, each responsible for a specific task. Let's start by moving the animation logic out into its own function.
From an encapsulation perspective, when we're performing a special attack, we don't need to know the inner workings of PlayerAnimationController and how it has a method PlayAttackAnimation(string animationName), so we can safely move this part out of the function without losing the details of a necessary part of the function.
// PlayerCharacterController.cs
public const float SPECIAL_ATTACK_RANGE = 30.0f;
public const float SPECIAL_ATTACK_BASE_DAMAGE = 5.0f;
public const float SPECIAL_ATTACK_TELEPORT_DISTANCE = 5.0f;
public const float SPECIAL_ATTACK_COOLDOWN_SECONDS = 1.0f;
private float _specialCooldownTimer = 0.0f;
public void PerformSpecialAttack()
{
if (_specialCooldownTimer <= 0.0f)
{
// Encapsulate animation logic to keep PerformSpecialAttack() focused on gameplay logic
PlayAttackAnimation();
RaycastHit[] hits = Physics.RaycastAll(transform.position, transform.forward, SPECIAL_ATTACK_RANGE);
EnemyCharacterController enemy = null;
for (int i = 0; i < hits.Length; i++)
{
enemy = hits[i].transform.GetComponent<EnemyCharacterController>();
if (enemy != null)
{
// deal damage
enemy.TakeDamage(SPECIAL_ATTACK_BASE_DAMAGE);
enemy.PlayHurtAnimation();
break;
}
}
if (enemy != null)
{
transform.position = enemy.transform.position - transform.forward * SPECIAL_ATTACK_TELEPORT_DISTANCE;
}
_specialCooldownTimer = SPECIAL_ATTACK_COOLDOWN_SECONDS;
}
}
// encapsulating animation logic to improve readability
private void PlaySpecialAttackAnimation()
{
PlayerAnimationController controller = GetComponent<PlayerAnimationController>();
controller.PlayAttackAnimation("special");
}
By moving the animation functionality into its own method, we simplify the original function by encapsulating unnecessary details.
Next, let's tackle another chunk of complexity: getting the first enemy in front of the player, dealing damage to them, playing the hurt animation, and teleporting to them. Right now, all this is coupled together, making it harder to update or debug.
To remedy this, we can again utilize the extract function technique by encapsulating the functionality for getting the first enemy hit, and using the results of that to perform the damage, animation, and teleportation.
// PlayerCharacterController.cs
public const float SPECIAL_ATTACK_RANGE = 30.0f;
public const float SPECIAL_ATTACK_BASE_DAMAGE = 5.0f;
public const float SPECIAL_ATTACK_TELEPORT_DISTANCE = 5.0f;
public const float SPECIAL_ATTACK_COOLDOWN_SECONDS = 1.0f;
private float _specialCooldownTimer = 0.0f;
public void PerformSpecialAttack()
{
if (_specialCooldownTimer <= 0.0f)
{
PlayAttackAnimation();
EnemyCharacterController enemy = GetFirstEnemyInFront();
if (enemy != null)
{
enemy.TakeDamage(SPECIAL_ATTACK_BASE_DAMAGE);
enemy.PlayHurtAnimation();
// teleport to enemy
transform.position = enemy.transform.position - transform.forward * SPECIAL_ATTACK_TELEPORT_DISTANCE;
}
_specialCooldownTimer = SPECIAL_ATTACK_COOLDOWN_SECONDS;
}
}
private void PlaySpecialAttackAnimation()
{
PlayerAnimationController controller = GetComponent<PlayerAnimationController>();
controller.PlayAttackAnimation("special");
}
// Encapsulate enemy detection to reduce complexity and allows for re-use in other parts of code
private EnemyCharacterController GetFirstEnemyInFront()
{
RaycastHit[] hits = Physics.RaycastAll(transform.position, transform.forward, SPECIAL_ATTACK_RANGE);
for (int i = 0; i < hits.Length; i++)
{
if (hits[i].transform.TryGetComponent(out EnemyCharacterController enemy))
{
return enemy;
}
}
return null;
}
After this refactor, our script is much more readable. But we're not done yet; there are still a few more refactorings we can do that can further ✨ beautify ✨ our code.
Again, remember that we should hide unnecessary details from our method. That includes the inner mechanisms of other functionality that is not directly necessary to our function. Some things that pop out are the cooldown timer reset, enemy damage, animation, teleportation. We can further apply extract function by extracting these into their own separate functions.
// PlayerCharacterController.cs
public const float SPECIAL_ATTACK_RANGE = 30.0f;
public const float SPECIAL_ATTACK_BASE_DAMAGE = 5.0f;
public const float SPECIAL_ATTACK_TELEPORT_DISTANCE = 5.0f;
public const float SPECIAL_ATTACK_COOLDOWN_SECONDS = 1.0f;
private float _specialCooldownTimer = 0.0f;
public void PerformSpecialAttack()
{
// guard clause to reduce nesting
if (_specialCooldownTimer > 0.0f)
{
return;
}
PlayAttackAnimation();
EnemyCharacterController enemy = GetFirstEnemyInFront();
if (enemy != null)
{
enemy.Hurt(SPECIAL_ATTACK_BASE_DAMAGE);
TeleportToEnemy(enemy);
}
ResetSpecialCooldownTimer();
}
private void PlaySpecialAttackAnimation()
{
PlayerAnimationController controller = GetComponent<PlayerAnimationController>();
controller.PlayAttackAnimation("special");
}
private EnemyCharacterController GetFirstEnemyInFront()
{
RaycastHit[] hits = Physics.RaycastAll(transform.position, transform.forward, SPECIAL_ATTACK_RANGE);
for (int i = 0; i < hits.Length; i++)
{
if (hits[i].transform.TryGetComponent(out EnemyCharacterController enemy))
{
return enemy;
}
}
return null;
}
// Extract teleportation logic to make potential future modifications easier
private void TeleportToEnemy(EnemyCharacterController enemy)
{
transform.position = enemy.transform.position - transform.forward * SPECIAL_ATTACK_TELEPORT_DISTANCE;
}
// Extract cooldown timer logic in case other parts of code requires it
private void ResetSpecialCooldownTimer()
{
_specialCooldownTimer = SPECIAL_ATTACK_COOLDOWN_SECONDS;
}
// in EnemyCharacterController.cs
public void Hurt(float damage)
{
TakeDamage(damage);
PlayHurtAnimation();
}
Quick summary of what we've done:
- Merged
enemy.TakeDamage(float)andenemy.PlayHurtAnimation()into a new functionenemy.Hurt(float) - Extracted teleportation logic into
TeleportToEnemy(EnemyCharacterController) - Extracted resetting the cooldown timer into
ResetSpecialCooldownTimer() - Utilized the guard clause refactoring technique to reduce the amount of nesting. See this article for information on that technique!*
Although the code has gotten longer, we've actually made the script as a whole much more readable, easy to maintain, and simple to debug. Whenever you need to revisit this code either to fix a bug or to add some new functionality, you'll spend a substantial less amount of time debugging because you know that each step of each function is properly encapsulated and explicity named. Need to update the teleportation logic so that it teleports the player in front of the enemy? Easy, we just need to update the TeleportToEnemy function! Is the player animation not being properly played? Without much reading, you can immediately guess that the problem might lay somewhere within PlaySpecialAttackAnimation!
You might have wondered: "why not just use comments?". But something you may have noticed is that as we refactored our code and created good descriptive names for the extracted functions, the need for comments disappeared! Reading through PerformSpecialAttack() is almost as clear as the specifications asked by your design team! While comments definitely do have their uses, they can actually hurt your code if they aren't constantly updated as your code changes. That's much less likely to happen by properly naming your functions since the name is tied to what the function does. By keeping your functions properly named, the functionality properly encapsulated, you avoid the unnecessary burden that they bring to your code.
Conclusion
Although going through an existing function and refactoring it may seem tedious, I hope that after reading this article you have an understanding of how important it is. As you write code, train yourself to spot opportunities for refactoring early. Small, well-structured functions not only make your code cleaner today but also save you countless hours debugging and extending it in the future. By utilizing the extract function refactoring technique, you can separate functionality so that it encapsulates unnecessary details away and makes each function step directly relevant to the task it's performing. This improves readability, makes your code easier to maintain, and bugs easier to find.
Hopefully you found this article helpful and are as excited as I am to continue writing beautiful code! 🚀