nullchecking - is this safe?

[movezig]

If you don't check for null it usually results in a server crash if you refer the the object that is null. So would a check like this be safe to do?

if ( talisman.Killer != null && talisman.Killer.Type != null )

Or should I nest this instead? This way is just cleaner, but I'm wondering if it will still check .killer.type even if the first one is null.

[mordero]

Yeah, checking if something is null like that wont cause any problems.

Edit: Correction: It only checks the second one if the first is false. See post below

[Kenko]

also, you should know that

if( statement1 && statement2 ){
    //stuff
}

works the same as

if( statement1 ){
     if( statement2 ){
           //stuff
     }
}

[movezig]

Thanks for the quick replies.

Originally Posted by Kenko

also, you should know that

if( statement1 && statement2 ){
    //stuff
}

works the same as

if( statement1 ){
     if( statement2 ){
           //stuff
     }
}

Yeah knew that, was just wondering as a statement2 when statement1 is null would result in a crash, so was not sure if checking both at once was safe or not.

[Lord_Greywolf]

you do need to check them seperate - i found this out the hard way

because of the 1st is null, then the type can not be found and will cause the crash

read my thread on the prospectors tool - that happened there - because the def was null, even checking for def.what ever being nul would crash

[Killamus]

Originally Posted by Lord_Greywolf

you do need to check them seperate - i found this out the hard way

because of the 1st is null, then the type can not be found and will cause the crash

read my thread on the prospectors tool - that happened there - because the def was null, even checking for def.what ever being nul would crash

Thats relitive, it only needs to be checked if they rely on eachother (For instance

if ( m != null && m.Mana >= 50)

would be a crash hazard, I do believe)

while

if ( from.Hits >= 50 && from.Mana >= 50)

wouldn't be a crash hazard.

[Kenko]

you are wrong Killamus, if( m != null && m.Mana >= 50 ) won't ever make a shard crash, simple as this:
*m is null*
code runs..
if( m != null....nope, m fucking null, ok stop already. it will never reach m.Mana if m == null.

[Killamus]

So then how do you figure the crash as stated by greywolf, if it doesn't reach it?

[mordero]

It only checks the second operand if the first one is false...
http://msdn2.microsoft.com/en-us/library/2a723cdk.aspx

Edit: Whoops forget to say what I wanted to. If you get an error because it is being null but still checking the second, you need to do a var != null on the first one (like movezig does), because then if it is null, it will return false, and thus wont check the second one.

[arul]

Originally Posted by Killamus

So then how do you figure the crash as stated by greywolf, if it doesn't reach it?

It works exactly as it should. The crash greywolf reffered to was caused by his wrong logic.

[Lord_Greywolf]

i would seperate them if they are a sub of the other one

if they are not related at all - then would be no problem (i,e, m.hits and m.mana is no problem - not a sub but if m != nul and m.mana i would)

because i had this crash from doing just that:

if ( root is PlayerMobile && ((PlayerMobile)root).ClassLevel <= 4)

gave a crash if it was not a player mobile - so even though 1st half was false - still checked the 2nd - giving the crash

[arul]

 if ( root is PlayerMobile && ((PlayerMobile)root).ClassLevel <= 4)

Post the exact exception this code throws. Either another part of your code is buggy or you just 'discovered' sort of .net bug. Seriously, I bet on the 1st option.

[daat99]

Originally Posted by arul

 if ( root is PlayerMobile && ((PlayerMobile)root).ClassLevel <= 4)

Post the exact exception this code throws. Either another part of your code is buggy or you just 'discovered' sort of .net bug. Seriously, I bet on the 1st option.

The only possible crash he can get from that is if "root" is null.

As for the rest of the argument:

if ( m != null && m.Mana >= 50)

This will NEVER crash in c# (it may on vb depends on your compilation settings).

[arul]

Nope, the 'is' operator at first checks if the operand is null. If so, the expression is evaluated as false.

[Lord_Greywolf]

mine crashed because the root was a packhorse and not a player mobile - and packhorses do not have class levels

i checked this out with statements being used before and after it to find the exact line causing the crash

the error was a null exception -- i do not have it saved to show exact wordage

i changed it to 2 lines (a sub if of the 1st) and then no more crash

[daat99]

Originally Posted by Lord_Greywolf

mine crashed because the root was a packhorse and not a player mobile - and packhorses do not have class levels

i checked this out with statements being used before and after it to find the exact line causing the crash

the error was a null exception -- i do not have it saved to show exact wordage

i changed it to 2 lines (a sub if of the 1st) and then no more crash

I'll look into it this evening and get you a definite answer about this.

Regardless my earlier statements is still valid (and if someone disagree than show proof!!! you won't find it...):

if ( m != null && m.Mana >= 50)

This will NEVER crash in c# (well you can change the "m" class to throw an exception if someone access the Mana property but this isn't the case here...).

[daat99]

Here's some testing code I made up to test your problem.
Just create a new console application project in visual studio 2003/5 (maybe in sharp develop also) and paste this code as it is on your main file:

using System;
//conclusion -> no such null
namespace testingNull
{
    class A { } //object that doesn't have "mana" property
    class B //object that have mana property
    {
        public B(int classLevel) { this.classLevel = classLevel; }
        private int classLevel;
        public int ClassLevel { get { return classLevel; } set { classLevel = value; } }
    }
    class Program
    {
        static void Main(string[] args)
        {
            A a = new A(); //doesn't have ClassLevel
            B b1 = new B(3); //have lower ClassLevel
            B b2 = new B(5); //have higher ClassLevel
            check(a); //object that doesn't have "class" property
            check(b1); //object that have lower class
            check(b2); //object that have higher class
            check(null); //"null object"
            check2(b1); //b that have lower class
            check2(b2); //b that have higher class
            check2(null); //"null b"
            Console.ReadLine();
        }

        private static void check( Object o )
        {
            if (o is B && ((B)o).ClassLevel >= 4) //object that have higher class
                Console.WriteLine("b and class higher than 4");
            else if (o is B) //object that have lower class
                Console.WriteLine("b and class lower than 4");
            else if (o is A) //object that doesn't have "class" property
                Console.WriteLine("not b - doesn't have class");
            else //"null object"
                Console.WriteLine("null");
        }
        private static void check2(B b)
        {
            if ( b != null && b.ClassLevel >= 4 ) //b have higher class
                Console.WriteLine("not null and higher class");
            else if ( b != null ) //b have lower class
                Console.WriteLine("not null and lower class");
            else //"null b"
                Console.WriteLine("null");
        }
    }
}

As you can see in the above example this code is 100% safe and will never crash a shard.

Dictionary:
root -> object
PlayerMobile -> B
Horse -> A
ClassLevel -> ClassLevel

Please note that nothing crashes regardless if the object have the ClassLevel property or it's null...

If someone disagree than show code that prove your point.

[Lord_Greywolf]

try a check2 on a - you did it with b's - which will be b's no matter what

and a defined null - defined as being nulls to start with, the system might treat deferently

it may also be something that runuo added in someplace along the line also
because i know that line caused the crash, replaced and no more crash

[daat99]

Originally Posted by Lord_Greywolf

try a check2 on a - you did it with b's - which will be b's no matter what

and a defined null - defined as being nulls to start with, the system might treat deferently

it may also be something that runuo added in someplace along the line also
because i know that line caused the crash, replaced and no more crash

Check2 expect to recieve B as the argument, sending it A instead makes no sense and serves no purpose (it won't even compile).
Before you try to drive a car you need to make sure you're in a car, not a ship...
What you did was getting into a ship and trying to drive it on the highway... makes no sense...

In any case this will not cause a null reference exception but some kind of "invalid cast exception" or "argument exception" (don't recall the name of that particular exception atm).

RunUO doesn't have its own scripting language, it uses c# itself.

**edit**
By all means, open visual studio, create a new "console application" project, copy the code I posted over the single file in there and test it.

[Lord_Greywolf]

it will not throw a null exception then, but can throw a crash still - because of diffefernt types - i dug the report out of my recycle bin

so even if the 1st part is invalde - it will still check the 2nd half and throw the error/crash

System.InvalidCastException: Unable to cast object of type 'Server.Mobiles.PackLlama' to type 'Server.Mobiles.PlayerMobile'.

so i would still place them on different lines for that reason - sometimes the 2nd half still gets checked, even if the 1st half is invalade

origional line that threw it:

if ( root is PlayerMobile && ((PlayerMobile)root).ClassLevel <= 4)

so even though root was not a playermobile - it threw the exception and crashed - so it checks the 2nd half also even through the 1st half was false

[arul]

Then you probably used a wrong operator. Replacing '&&' with only '&' is a common mistake.

[David]

Originally Posted by daat99

if ( m != null && m.Mana >= 50)

This will NEVER crash in c# (it may on vb depends on your compilation settings).

I have to agree with daat99 on this one...

[Lord_Greywolf]

Originally Posted by arul

Then you probably used a wrong operator. Replacing '&&' with only '&' is a common mistake.

did you read the line that threw the exception - it is an && there and not just an & - it would not eeven compile if i used just an & there

it may not check the 2nd half on a null check, but it does when it is not a null check