Announcement

Collapse
No announcement yet.

Check for null pointer, good pratice ?

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

    Check for null pointer, good pratice ?

    Hello,

    I have been wondering for some time why does everyone check everywhere if a pointer is null before using it, here is an example from the tuto I'm was following:
    Code:
    if (GEngine)
       GEngine->AddOnScreenDebugMessage(-1, 5.f, FColor::Yellow, TEXT("BeginPlay"));
    The thing is, besides it's often less than unlikely that these tested-pointers would be null, also, of course the application will crash if it's not checked, but isn't it easier to crash and immediately know what's going on rather that having this WHY-IS-NOTHING-HAPPENING feeling ?
    can't imagine the mess with an huge code base if something start to not working for no apparent reason, and then have to use the debugger "if" by "if" in the whole application to finally figure out which pointer is now null...

    Even if it's not really unreal engine related, my question is, why the hell don't you let it crash ?
    Last edited by Titan.; 08-29-2014, 07:11 PM.

    #2
    Even if it's not really unreal engine related, my question is, why the hell don't you let it crash ?
    It makes for a very bad user experience for starters.

    That's kinda okayish for development but not ideal for production. It could be possible that it's never null for you but a user finds some edge case that causes your pointer to be null.

    If you needed that pointer to be valid and it's not, then you should handle that condition. That should mitigate any weird bugs. Look at what you're doing with the pointer and judge what the implications are. In your example it's not a big deal - you can just log to a text file or just skip it. In other cases you may need to gracefully fall back - for example don't spawn a character and play an error sound.

    You should _always_ check pointers that are passed in or ones you get from calling a function - eg GetWorld(); You just never know what their state is frame to frame.

    There will be times when you're certain it'll be a valid pointer (ie created an object in your class fully controlled by you). UE4 doesn't check them all. UE4 also uses asserts in some areas.

    Anyway, ultimately crashing makes for a terrible user experience. Gracefully handle null pointers.

    Would you put up with the UE4 editor crashing randomly because that's how Epic catches bugs?

    Comment


      #3
      Checking GEngine pointer in game-code is useless, since engine will crash much earlier before check point.
      How Bino noted, that's an awful way to catch bugs - just let it crash. You should always check pointer that passed to the function, or the one that was cast to another type and so on, since you can't be sure it's not null.

      Regarding debugging and nothing happening feeling, that's why you don't simply check pointer but print some information and have an external way to quit execution, instead of crashing.
      Alone: The Untold - a story driven horror game

      Comment


        #4
        I use copy of default check() macro.

        Code:
        #define GAMENAME_DEBUG 1
        
        #if GAMENAME_DEBUG
        	#define GAMENAME_CHECK(expr) { if(!(expr)) FDebug::AssertFailed( #expr, __FILE__, __LINE__ ); CA_ASSUME(expr); }
        #else
        	#define GAMENAME_CHECK(expr)
        #endif
        Code:
        GAMENAME_CHECK(Pointer)
        
        if(Pointer)
        {
            // Do something
        }
        This solutions causes crash only when GAMENAME_DEBUG is 1. So when I ship my game I set it to 0 and end users will not experience asserts

        Comment


          #5
          Well, there are better ways than just checking for nullPtr's.
          You could use the Nullobject-pattern or exceptions as an elegant way to deal with the problem and actually provide feedback to the developer or the user if needed.

          BUT YOU NEVER EVER WANT TO ACCESS UNCHECKED POINTERS! Nothing is worse than reproducable crashes that only happen on 1-2 system setups. If you failed to test this specific setup, you practically ship a game that could just crash on every non-tested computer/setup.
          Last edited by DennyR; 08-30-2014, 11:35 AM.

          Comment


            #6
            Sorry to res this thread. But I found this just then.
            Best to always check GEngine

            Code:
            /** Global engine pointer. Can be 0 so don't use without checking. */
            extern ENGINE_API class UEngine*			GEngine;

            Comment


              #7
              Originally posted by Bino View Post
              Sorry to res this thread. But I found this just then.
              Best to always check GEngine

              Code:
              /** Global engine pointer. Can be 0 so don't use without checking. */
              extern ENGINE_API class UEngine*			GEngine;
              Well, it's always better to check pointer before using, but

              Code:
              if (GEngine)
                 GEngine->AddOnScreenDebugMessage(-1, 5.f, FColor::Yellow, TEXT("BeginPlay"));
              But this piece of code is executed in actor's BeginPlay function, as i see, there is no way GEngine could be null by this time, i believe
              Alone: The Untold - a story driven horror game

              Comment


                #8
                Originally posted by BiggestSmile View Post
                Well, it's always better to check pointer before using, but

                Code:
                if (GEngine)
                   GEngine->AddOnScreenDebugMessage(-1, 5.f, FColor::Yellow, TEXT("BeginPlay"));
                But this piece of code is executed in actor's BeginPlay function, as i see, there is no way GEngine could be null by this time, i believe
                Belief is not knowledge, however. If the API contract states that a given value can be null, then it can be null at any time, and you have to check for that. Even if you think you know that it can't be null at a given point in time, because you cannot be certain that that behaviour will stay that way in subsequent revisions.

                Comment


                  #9
                  Originally posted by The_E View Post
                  Belief is not knowledge, however. If the API contract states that a given value can be null, then it can be null at any time, and you have to check for that. Even if you think you know that it can't be null at a given point in time, because you cannot be certain that that behaviour will stay that way in subsequent revisions.
                  Yup. Also, I've had some strange cases in the past when GEngine was in fact, and unexpectedly null.

                  Comment

                  Working...
                  X