In a lot of functions I could simply reduce them to one line if I wanted to, but due to overly checking pointers this sometimes becomes 7+ lines.
When is it appropriate to check UWorld/GEngine/etc for non-nullptr-ness? At the moment I’m doing it pretty much whenever I need them which just seems inefficient.
Thanks.
I check pointers every time. The only time you don’t need to check them is when you are 1000% sure, that it is valid.
One time, i had someone who got a problem with a not respawning Player. But that only happened about 50% of the time.
Pretty random. So i looked into his code and saw, that he never checked if the spawned Player was really valid and just assumed
that the spawn worked. Since then, i check nearly every pointer i make.
Also in BP i use the “IsValid” node on every pointer/reference variable. You never know if something is created a few miliseconds
after your call.
I check pointer only when 100% sure that it may be invalid and I know what to do when it is invalid. The reason for that is when you assume that this particular pointer should be valid, but it is not, then you have a problem somewhere else. It is easier to look at crash place than manually stepping with debugger through hundreds of “if(myPointer){}” to find which one is zero. That tactic saved me a ton of time figuring out why some part of my code isn’t working as I think it should.
I honestly never check GEngine since it’s usually initialized before anything else possibly happens (by GameInstance). At least in regular code anyway. If I was changing something in GameInstance I probably would check it.
Anything in game-code though, I always check it without fail. Just isn’t worth the hopeless debugging if it causes a crash one day!
I have to agree with Druha on this one. While the idea of ‘always checking for null’ may seem like good practice, I think it can actually lead to a way of thinking about error state handling that isn’t good and can result in more difficult to track down problems.
As Druha says, it really comes down to what you know and can expect about the input values to functions, or the return values of functions you are calling; along with how you intend to handle an undesired null pointer. If null is a valid value for a variable to have at a particular point in code, and you understand why it might be null, then you absolutely should check and handle accordingly.
The practice I disagree with is wrapping blocks of code inside a null pointer check just ‘to be on the safe side’, when you aren’t aware of if, when and why the pointer could be null. This isn’t so much handling an error case as pretending it’s not there - instead of causing an exception so you can debug and find out why it was null, you just omit doing what you wanted to do. In some cosmetic cases this might be fine, but generally, if you’re expecting your code to get executed, not executing it because of an undesired null pointer is likely to have adverse consequences further down the line.
I’d recommend using the check and ensure macros provided in UE4.
/* This is essentially the same as not doing a null check, but is a bit more explicitly self documenting. */
check(InPointer);
InPointer->DoSomething();
/* This is wrapping code in a null check, but explicitly showing that a null pointer is an unexpected state that needs investigating. The debugger will break here on a null pointer, but allow you to continue running. */
if(ensure(InPointer))
{
InPointer->DoSomething();
}
Bottom line: try to always understand when and why a given pointer might be null and deal properly with each case, rather than relying on a catch-all fallback. Admittedly, with the lack of C++ documentation of UE4 API functions, this can be difficult in practice.
I agree with kamrann and Druha - I want to know during development as soon as something is nullptr when I didn’t think it possible. Obviously some stuff will, for example, return nullptr when it fails internally, and that can usually be handled, but again I don’t always want to ‘fail softly’. It’ll probably be easier to fix bugs if all of them are hard crashes - it’s the softies you’ve got to worry about!
@kamrann; I love the ‘ensure’ check in your example. Throwing breakpoint + being able to continue is awesome, and I’ll be using that one more from now on. Thanks!
Actually yeah I should have added to that. We / I use an ASSERT library that overrides the Engines Check and CheckF functionality to not crash the entire editor when something goes wrong in Gameplay code. We get a nice error message and an Excel spreadsheet etc. with details.
If I know that a pointer should never be null at a certain point, I check with an assert instead.
I’m new to C++ programming and discovered that even IsValid can cause a crash if you haven’t initialized a pointer at all. I have a struct with pointers as function arguments, and I didn’t declare where to pass the variable. It might pass even a simple check (Pointer != nullptr) and return true because it’s not nullptr, but it’s also not a valid pointer — I don’t know what it is. So, initializing structs with pointers to at very least make them nullptr seems like a very good practice, I suppose?
Yes thats good practice. Our code standard is (1) initialize any pointer variable to nullptr and (2) nullptr check any pointer being used.
Could you provide some details about your ‘ASSERT library’? I think this approach is much better than the endless crashes that come with Unreal Editor.