Is it a code smell to check game object references for null to see if they've destroyed themselves?

If you have a collection of game objects which persists, and some of those game objects can destroy themselves, you can check for null references later to detect that fact. Or you could have the ready-to-be-destroyed objects send a message (via event or whatever mechanism) to let the collection-holder know. Is using null checks a code smell? In general, which approach do you prefer and why?

1 Like

Checking a list for null values is a completely normal way of handling this that I’ve seen around, but I prefer one of two other options.

First, make a “destroying object” event that anything can subscribe to (different events for different types of objects) so that anyone who’s interested can get the message and deal with their lists/references themselves.

Second, the quick-and-dirty way: have some manager (maybe your “collection-holder”) deal with removing references by calling a function. The manager can also take care of the actual object destruction, if you want.

EDIT: To clarify, in the former option I would make a Singleton class to handle events and the “destroying type of object” event could be subscribed to from all over, and each receiver would receive notice of every object of that type being destroyed and have to check internally if the object is relevant to them. I’m not suggesting that every object in the game should have an internal event handler that can be subscribed to- although that would also be an option I suppose.

2 Likes

I prefer the about-to-be-destroyed object to send a message because this is event driven and can have a positive impact on performance. I wouldn’t call the alternative you suggested a code smell though, it’s just a different approach with pros and cons.

Sending a message might not be possible because you don’t own the code which does the destroying: GameObject.Destroy(). Not to mention, sending a message probably wouldn’t be possible if GameObject.DestroyImmediate() were called.

1 Like

Not at all. It only takes a single line of code to remove all null references from a collection.

As a general rule objects should not know about things that have a reference to them, including what collections they are in. Circular references are generally to be avoided.

If this is in hot code and you need uber performance, then by all means, set up an event system. But this is more boiler plate code that will be required. The performance gains will only make sense in some limited situations.

1 Like

Thanks for all the thoughts, which make sense.

Following further research, I still suspect it IS a code smell, but more due to specific problems with Unity’s implementation, i.e. that interfaces don’t work with the null check and Unity’s own analysis that the null-check approach is a “counterintuitive” design with other flaws – one which they have seriously considered replacing.

1 Like