CODEWITHSUNDEEP
c#
xNidhogg
xNidhogg

Reputation: 1

Why does my code ignore a true if-condition?

I've got the following code:

    private static void checkCodesInPlayerCenter(GameObject player)
    {
        Vector2 collisionCenter = player.GetCollisionCenter(player.PublicCollisionRectangle);

        if (TileMap.GetMapSquareAtPixel(collisionCenter) == null)
        {
            return;
        }

        for (int i = 0; i < TileMap.GetMapSquareAtPixel(collisionCenter).Codes.Count; ++i )

It is possible that sometimes the object I get from GetMapSquareAtPixel is null. To not cause a NullReferenceException in the for-loop I decided to check is its null, and if so end the function early, however it seems to completely ignore the if-condition, even if the object returned is null. I've set a breakpoint on the return statement but the code never goes there and instead triggers the NullReferenceException I tried to avoid.

Any help please?

Upvotes: 0

Views: 161

Answers (5)

theotherian
theotherian

Reputation: 189

If the method you're calling is mutating state (which generally a getter shouldn't do), then it could be that it's not an idempotent call, meaning it's not safe to call more than once.

Additionally, adding that expression in the debugger is quite possibly causing it to be evaluated before you step into your code, resulting in the NPE.

I would advocate trying to store the value of TileMap.GetMapSquareAtPixel in a separate variable.

Upvotes: 0

Tigran
Tigran

Reputation: 62256

I strongly suggest to assign TileMap.GetMapSquareAtPixel return value to some local variable and after check it against null condition and use the same var in for loop.

This will work.

Upvotes: 0

anthony sottile
anthony sottile

Reputation: 69944

Is it possible the return value of the function changes between the two conditions?

Try assigning to a local variable and doing your check (and step through with the debugger)

private static void checkCodesInPlayerCenter(GameObject player)
{
    Vector2 collisionCenter = player.GetCollisionCenter(player.PublicCollisionRectangle);

    var squareAtPixel = TileMap.GetMapSquareAtPixel(collisionCenter);

    if (squareAtPixel  == null)
    {
        return;
    }

    for (int i = 0; i < squareAtPixel.Codes.Count; ++i )

This is also possibly better if the GetMapSquareAtPixel call is expensive (one call vs two calls when all you really care about is a null check)

EDIT: Which variable is it attempting to get at when the NRE is thrown?

Upvotes: 0

Random Dev
Random Dev

Reputation: 52280

what if the function returns two different results (unlikely but don't call this possible expensive function twice anyhow - save the result in a variable and check/use this variable)

What if the .Codes part is null - check this as well!

Upvotes: 1

ChrisF
ChrisF

Reputation: 137148

It's probable that

TileMap.GetMapSquareAtPixel(collisionCenter)

is not null, but

TileMap.GetMapSquareAtPixel(collisionCenter).Codes

is. If that's the case then

TileMap.GetMapSquareAtPixel(collisionCenter).Codes.Count

will fail with NullReferenceException.

You need to add that to your guard conditional:

    if (TileMap.GetMapSquareAtPixel(collisionCenter) == null ||
        TileMap.GetMapSquareAtPixel(collisionCenter).Codes == null)
    {
        return;
    }

Upvotes: 2

Related Questions