Just how bad is it to use GetComponent?

Unity Engine
1

Hello!

Sometimes when writing scripts quickly, it’s tempting to write something like this:

IEnumerator DamageTimer(Transform AttackingSoldier, Transform DefendingSoldier)
{
    yield return new WaitForSeconds(AttackingSoldier.GetComponent<HealthInfo>().DamageWaitTime);
    DefendingSoldier.GetComponent<HealthInfo>().TakeDamage(AttackingSoldier.GetComponent<HealthInfo>().DamageAmount);
    Debug.Log("Done! " + Time.time);
}

My question is: Is this bad practice, and if so, why and how to get around it? Is it better to get components in Start() and store them as a var?

Thanks for any ideas :smile:

2

It is considered bad practice to constantly get component multiple times, like in Update or FixedUpdate. There are special exceptions to the rule (for example if the said component is expected to constantly get added or removed at runtime), however even under those exceptions there’s usually a better way to do it.

in your case I’m seeing a different bad practice, reckless Object Chaining. What if the Defending Player doesn’t have a HealthInfo component? you’ll get a null ref the instant you try to call TakeDamage.

try avoiding Object chains unless the said methods always return themselves (or a non-null, like Linq methods, …ooor if you’re chaining ExtensionMethods which can handle null references). its better to break up that code into multiple lines and also check for null on each result, which will also make the code easier to breakpoint. plus in Coroutines also assume that those results can be null after a yield so check them again after.

2 Likes