So, let’s say you have a reference in script A to script B, and you want to call B.method() from A. Let’s compare these three scenarios:
Before calling B.method() you check if B is null. If it is, you log a warning or an error.
Before calling B.method() you check if B is null. If it is, you do nothing.
You do not check if B is null at all.
In scenario 1, you check for a null error and let the user know. In scenario 2 you check for an error but don’t inform the user. In scenario 3 you don’t check and don’t EXPLICITLY inform the user.
My point is, scenario 1 is supposed to be the best, but in scenario 3 you actually inform the user, because Unity is going to automatically call a ‘LogError’ for the NullPointerException. Also, you don’t get any unwanted behaviour either because Unity handles it gracefully without crashing.
This last statement is the only thing I’m not sure about, can null errors impact the application behaviour in any way? I’ve never seen that happening (in Unity) but I don’t have enough knowledge about Unity’s inner workings to be sure.
If it doesn’t cause any problem, then checking for null and logging an error (scenario 1) is literally replicating a functionality that Unity already implements on its own, so it makes no sense. While checking for null but logging nothing (scenario 2) can actually be detrimental, since it can hide assignation errors and lead to a worse debugging experience. This leads me to believe that not checking for null at all is in effect a good programming practice (in Unity!).
What do you think? Can I get some insights into how Unity handles null exceptions internally?
Yes they impact application behavior in a very specific way - an exception is thrown and the call stack will be unwound up to the point where something catches that exception - most likely the Unity Engine itself if your code isn’t doing it. So your code from that point on will stop executing.
Unity itself will not crash, but there are concrete disadvantages to allowing an uncaught exception to “fly free” so to speak. One is that you are stuck with the “the rest of your code simply will not execute” behavior. Sometimes that’s what you wanted, but very often it is not what you wanted at all. You will often want to say “if this is null, do this”, and continue executing. Without the null check you won’t get that chance, your whole call stack will be unwound and Unity will go on with the next Update() or whatever method it’s going to call.
Another is that exception handling is not free. Unwinding the stack and checking for try/catch statements has a cost to it. It would be much cheaper computationally to simply do a null check.
In general checking for null is something that should be done when an object is reasonably expected to be null during the course of normal program operation. Obviously do not put null checks for references that you do not reasonably expect to be null.
My general philosophy: if the reference is of the “if this reference isn’t set then nothing will work” variety, I’ll leave it naked - I WANT that particular null reference error, so I can just fix it right away.
If I’m making a system that calls other code which is meant to be expanded on later, I ALWAYS wrap those calls in a try/catch and spit it out with Debug.LogException, so exceptions in subsystem A don’t break subsystems B through D.
Any typing, file, network, or hardware input should always be wrapped in try/catch with full reporting/logging, because anything can happen with those things. I generally make these logs far more detailed (including much more context around the exception) because these kinds of exceptions are often harder to reproduce, so you want to get as much information around it as possible right away.
Usually I try to make all methods return a default value if some part is null, so that in the worst of cases the game does something funky but eventually recovers from the glitch. I also put a Warnning stating what exactly was null, and what did you fail to assign or pass. I’m not used to try/catch blocks, but I write something that is kind of similar. In every method I always have a “Check” region, were I make sure that everything needed is there.
If the null comes from other place that is not the parameters, then you have a design error that leads to the type of design flaw “spooky action at distance”. You should never have any method depending on something that is not pass in the parameters or created inside the method. Sometimes you may use a global variable, but that kind of thing is really dangerous and you should avoid it.
Also, if the code won’t work without the reference I use a property that if null assigns the value.
So, basically I think you should always check for null, it’s worth it.
IMO it depends on what the specific code you’re referring to intends to do.
I also think there are other possible things to do than your 3 options.
1) For example I would throw my own exception in a situation where a object is definitely expected. Lets say I have an API that I want to have informed exceptions for. Why I may want informed exceptions is because I’m on a team, or I distribute this API, or I just really like keeping myself informed because I forget what I wrote.
Take this code:
public static void Shuffle<T>(T[] arr, IRandom rng = null)
{
if (arr == null) throw new System.ArgumentNullException(nameof(arr));
if (rng == null) rng = RandomUtil.Standard; //ignore this line, I'll be getting to this later
int j;
T temp;
for (int i = 0; i < arr.Length - 1; i++)
{
j = rng.Next(i, arr.Length);
temp = arr[j];
arr[j] = arr[i];
arr[i] = temp;
}
}
Here I want to throw an exception so that way we know something is wrong, but also how to fix it.
If I didn’t throw the exception the for loop is going to get a nullrefexception. Another developer won’t necessarily know what they did wrong. Someone astute enough to go and look at the code would say “oh, I need an ‘arr’ to operate on”… but if hanging around these forums has taught any of us anything. People aren’t generally that astute.
Heck, in my enterprise job I guarantee you there are teammates of mine who would go in there and think ‘Shuffle’ is bugged and try to “fix” the method. Instead of realizing they called it with the wrong parameter.
Where as by throwing the ArgumentNullException we’re explicitly stating “hey, you need a non-null argument”. Of course this may go over some heads as well. But at least it’s explicit.
2) This is a viable option if this is the behaviour I want.
Lets take this code for instance:
void OnTriggerEnter(Collider other)
{
var c = other.GetComponent<SomeScript>();
if (c == null) return; //we entered an unsupported trigger
c.DoStuff();
}
In this situation the checking null and doing nothing is expected behaviour. If the collider doesn’t have a ‘SomeScript’… I don’t do anything. Otherwise I ‘DoStuff’.
2.5) This leads me to one of my alternate scenarios.
Back to this:
public static void Shuffle<T>(T[] arr, IRandom rng = null)
{
if (arr == null) throw new System.ArgumentNullException(nameof(arr));
if (rng == null) rng = RandomUtil.Standard;
int j;
T temp;
for (int i = 0; i < arr.Length - 1; i++)
{
j = rng.Next(i, arr.Length);
temp = arr[j];
arr[j] = arr[i];
arr[i] = temp;
}
}
In here I’m letting “null” represent some default value. So I want to check if ‘rng’ is ‘null’ and if it is set it to my default.
It’s defined behaviour where ‘null’ means a thing, and so therefore I do that alternate thing based on if it’s null.
3) I personally avoid this situation honestly.
If I have a variable I expect to be a value and it potentially isn’t a value. I resolve for that.
This means that if I get a "NullReferenceException’ I see that as “oh, there’s somewhere that I didn’t account for all potential situations”.
Lets go back to the OnTriggerEnter, but remove the null check:
void OnTriggerEnter(Collider other)
{
var c = other.GetComponent<SomeScript>();
c.DoStuff();
}
If I had written this code there is 2 options here:
A ) I wrote this expecting to make sure ANY AND ALL Collider’s in my game that are triggers has a ‘SomeScript’ on it.
B ) I made a coding mistake and forgot to rule out trigger Collider’s that don’t have ‘SomeScript’.
The first is very deliberate and a restriction on the design of my game. I’d also argue it’s a really weird restriction since I have no way to actually enforce it at design time (what stops someone on my team from creating a trigger Collider without SomeScript attached?).
So unless I figured out a way to enforce this… I would likely throw a completely different exception (maybe even only at editor time) like so:
void OnTriggerEnter(Collider other)
{
var c = other.GetComponent<SomeScript>();
#if UNITY_EDITOR
if (c == null) throw new System.InvalidOperationException("A trigger Collider exists in the world without SomeScript attached. Please ensure all trigger Collider's are configured to the game specifications.");
#endif
c.DoStuff();
}
The 2nd is the more likely situation.
I made a mistake.
And the NullRefException is letting me know that I made a mistake as the programmer.
…
Basically all the options you outlined are all potential and meaningful things to do depending on what you intend to do.
The issue is determining “what do you intend to do with this bit of code?”
If you have a ref type that could potentially be null, ask yourself “well, what do I want to happen if it’s null?” Beyond that… the 1 vs 3 thing comes down to how descriptive do you want to be about a failed situation? I feel 1 is more descriptive and lets anyone working with your code (including yourself) understand exactly what mistake they made. By doing so the developer is conveying to others “I wrote this knowing this situation can occur and if it does this is the configuration mistake made that led to it and how to resolve it”. Where as 3 is just blind and puts the pressure on the developer reading your code to solve the puzzle of “why is this happening?”
But that’s all just my opinion.
And it comes from years of writing enterprise code with teams of people ranging from jr to sr. Writing expressive code is my primary goal.
In video games a lot of these things may go to the way side especially if they become performance bottlenecks. Since in game dev, performance is king.
And lets say doing some null check every single OnTriggerEnter script over 1000 entities that are constantly hitting one another was causing a 1% performance lost. So removing it may be a better choice. In that situation I may do something like this:
void OnTriggerEnter(Collider other)
{
var c = other.GetComponent<SomeScript>();
//NOTE - it was determined this null check was a performance issue. Removed and leaving note that if you end up here with a nullrefexception, you must fix the problem.
//if (c == null) throw new System.InvalidOperationException("A trigger Collider exists in the world without SomeScript attached. Please ensure all trigger Collider's are configured to the game specifications.");
c.DoStuff();
}
This brings up some interesting points and as ever it’s not always clear or there are multiple options. For some of my code that I’ve been working with I’ve been doing null reference checks for script components on gameobjects in particular. I do this as depending on the timing of the code being executed it can sometimes throw up an exception error before the block of code has successfully finished leading to some constant annoyances depending on what’s going on.
So for instance, I have some functional code whereby I’m checking for the existence of a village store and specifically the script attached to it before I execute any code. If that code is null I then use an else statement to not execute any code while I work on the alternative code so that I don’t throw up any errors that interfere with the overall game because my project is that big now. In this case, I’m checking, does the village store exist? If so then assign the village store script to the appropriate declaration, if the village store does not exist, then stop that particular block of code. This then lets me think about how I’m going to handle the else statement which is going to search for the alternative stockpiles I’ll have in the game that operate in place of the village store.
I think as long as you’re sensible and use logical if statements you’ll be fine but of course other people with more experience than me will have different opinions. I would say the main thing is of course just make sure you’re not calling null checks each frame for the sake of efficiency but that’s a general rule anyway when it comes to that sort of thing.
In Unity checking unity objects for null is costly. Avoid in runtime as much as you can. Do it through (unit) tests and editor (or compile) time checks whenever you can get away with those.
I might be tempted to have a look at multiple methods as well just to properly test how much it affects performance, problem is as I pointed out in my previous post I encountered some annoying errors popping up because of how fast the code was executing and I’ve been thinking about that quite a bit lately and how to deal with it.
One thing I never understood is why the compiler can’t say not only the line, but also the character were it found the null. Other compilers can do this. It would be much easier if you knew what exactly was null.
Because it’s not a compiler error but a runtime error. So it’s the compiled and jitted native code that does perform the null check at runtime and throw an exception if you try to dereference a null value.
A single line of source code can contain several parts, statements and expressions. Those are ultimatively broken up into several actual op codes that run on a cpu. Since the mapping between native code and the managed source code is kinda fuzzy, the debug information often only contains line information. So the compiler remembers “I converted this line of source code into that blob of op codes”. So when an exception is thrown in that blob, it came from this line.
What do you think is actually executed first? The general code structure of that line could be broken down into those general sequential sections
var someValue = objectC.GetComponent<ClassD>().someValue;
var newObject = new ClassB(someValue);
objectA.SomeMethod(newObject);
So it’s already kinda backwards. However we’re not done breaking it down. The first part also has be be broken down so the overall highlevel code would be
var classDInst = objectC.GetComponent<ClassD>();
var someValue = classDInst.someValue;
var newObject = new ClassB(someValue);
objectA.SomeMethod(newObject);
Each of those parts could throw an exception and we’re still in highlevel code. Each of those broken down lines translate to several instructions. So the runtime would need to keep track of every relationship of every piece of code and to which character of source code it belongs to. This isn’t always that easy since many highlevel constructs are actually converted into something else where a 1-to-1 relationship is not always possible. Think of a foreach loop. Linq expressions, Anonymous functions / Lambda expressions.
Compiler errors usually tell you the exact line and column since the compiler actually deals with the high level source code it compiles. At runtime that relationship is very blurry. For example, if you get an array index out of bounds exception, which character should the error point to? The first character of the expression that represents to wrong index? Or the last character of that value? Or maybe the array that is indexed? What about
var val = objectA.myArray[objectB.indices[42], objectB.indices2[42]];
Technically the outer array reference / value is this expression: objectA.myArray. Imagine the first index is out of bounds. The whole index is represented by this expression objectB.indices[42]. What if the 42 is already out of bounds and not just the value stored in the indices array?
I don’t want to imply that it’s impossible to create such a mapping for debug purposes. However it does not only put heavy constraints on compiler and jit compilers since they always have to generate this debug information for every piece of code, but in a lot cases it would be hard to interpret by the programmer correctly.
The takeaway here is: Don’t write such convoluted long lines of code (Kurt would call them “hairy” lines of code). Break them up into actual readable pieces. This has the additional benefit of adding implicit documentation. As you store intermediate results in variables, you have to name them accordingly. So the code is easier to read and to debug.
Even though few people here seem to agree, it’s a long established best practice to crash (throw unhandled exception) early instead of handling all errors where ever they can occur.
So if possible, do the null checks at the highest possible level, or not at all.
If your code is architected right, there’s no way the value can be null.
If you fear it might if it mustn’t, use an assertion for code and values you control, and write a null check for code you don’t control.
You guys sound like a reference being null means an error has occurred. With the way Unity works, I frequently use null checks as part of just normal code.
For example, if I write an enemy targeting script, I’ll check each frame if if target is null, because the target could have been destroyed. If null, I’ll find a new target, if not null I fire on the current target.