Refactor lines of code, decrease speed?

Hey all - mark this day - this is the first time on the internet I ever commented on anyone’s code ever in over a decade. Trying to step out of my shell a bit.

This is absolutely not a criticism because I often get stuck between these crossroads and wondering what the typical thought process is behind choosing. My best guess is that because it’s under Spawn and we know we aren’t being called in something like an Update, it’s relatively rare to run and we can pick our fights elsewhere.

So…granted casting should be fast enough, isn’t it true that during the video when Sam refactors the if {}…else if{} that he is adding an unnecessary cast to be done every single time even if animatorOverride is not null?

The first line in the refactored code always runs:

            //This line always runs:
            var overrideController = animator.runtimeAnimatorController as AnimatorOverrideController;
            if(animatorOverride != null)
            {                
                animator.runtimeAnimatorController = animatorOverride;
            }
            else if(overrideController != null)
            {
                animator.runtimeAnimatorController = overrideController.runtimeAnimatorController;
            }

Whereas previously, it only ever runs when it absolutely needs to:

            if(animatorOverride != null)
            {                
                animator.runtimeAnimatorController = animatorOverride;
            }
            else
            {
                 //This will only run when it needs to:
                var overrideController = animator.runtimeAnimatorController as AnimatorOverrideController;
                if(overrideController != null)
                {
                    animator.runtimeAnimatorController = overrideController.runtimeAnimatorController;
                }
            }  

Bonus question: Is this considered being picky? Haha. I feel like I wasted my first ever comment on something that in the grand scheme isn’t huge. But in general, just wondering about best practice and the pros/cons vs the else if taking up the extra line.

1 Like

Being able to evaluate a fragment of code you didn’t write and spot an improvement is an important skill. That being said, it’s always important to check for unwanted side effects when refactoring.
In this case, I see no unintended side effects from this particular refactor. The end effect will be the same, while preventing the temporary allocation of an object reference on the heap.

In terms of priority, I would put this refactor as a lower priority than a similar fragment of code in an Update loop.

Privacy & Terms