Which is correct way to check for Null exception?

Question

Which is the most correct code?

if (HttpContext.Current.Response.Cookies[authCookieName] != null) {
    HttpContext.Current.Response.Cookies[authCookieName].Value = "New Value";
}

or

if (HttpContext.Current != null)
    if (HttpContext.Current.Response != null)
        if (HttpContext.Current.Response.Cookies != null)
            if (HttpContext.Current.Response.Cookies[authCookieName] != null)
                HttpContext.Current.Response.Cookies[authCookieName].Value = "New Value";

Answer 1

If any one of HttpContext, HttpContext.Current, HttpContext.Current.Response, or Http.Current.Response.Cookies is null, you're already in trouble. Let the exception happen and fix your web server.

Answer 2

Neither is really more correct, though I would avoid the second, as deeply nested conditionals tend to be hard to understand and maintain.

If you would prefer you get a null pointer exception, use the first. If you want to deal with nulls in another way or silently, use the second (or a refactored version of the second).

Answer 3

If you think there's a chance that Current, Response, Cookies, or Cookies[authCookieName] could be null, and you have a reasonable thing to do if any of them are, then the latter's the way to go. If the chances are low, and/or there's nothing you can do if the intermediates are null, go for the former, as it's more concise - the best you could do is to get better logging if you use the expanded example.

Answer 4

could try:

if(HttpContext.Current != null && 
   HttpContext.Current.Response != null && 
   HttpContext.Current.Response.Cookies != null && 
   HttpContext.Current.Response.Cookies[authCookieName] != null) 
{
    // do your thing
}

Answer 5

HttpContext.Current.Response.Cookies will never be null. The only thing that can cause a null is if the cookie you are expecting doesn't exist, so the first is correct. HttpContext.Current would be null if you weren't accepting a web request though :)

Answer 6

Both are good. Assuming that you have already checked everything else that need to be checked first. E.g.:

private bool CheckSuspendersAndBelt()
{
    try
    {
        //ensure that true is true...
        if (true == true)
        {
            //...and that false is false...
            if (false == false)
            {
                //...and that true and false are not equal...
                if (false != true)
                {
                    //don't proceed if we don't have at least one processor
                    if (System.Environment.ProcessorCount > 0)
                    {
                        //and if there is no system directory then something is wrong
                        if (System.Environment.SystemDirectory != null)
                        {
                            //hopefully the code is running under some version of the CLR...
                            if (System.Environment.Version != null)
                            {
                                //we don't want to proceed if we're not in a process...
                                if (System.Diagnostics.Process.GetCurrentProcess() != null)
                                {
                                    //and code running without a thread would not be good...
                                    if (System.Threading.Thread.CurrentThread != null)
                                    {
                                        //finally, make sure instantiating an object really results in an object...
                                        if (typeof(System.Object) == (new System.Object()).GetType())
                                        {
                                            //good to go
                                            return true;
                                        }
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
        return false;
    }
    catch
    {
        return false;
    }
}

(sorry, couldn't resist... :) )

Answer 7

The first example you gave is more than enough. Like mentioned, if any of the other objects are null there is a problem with ASP.NET.

if (HttpContext.Current.Response.Cookies[authCookieName] != null) {
    HttpContext.Current.Response.Cookies[authCookieName].Value = "New Value";
}

But rather than littering your code with these often many checks, you should create some generic functions like SetCookie, GetCookie, GetQueryString, and GetForm, etc. which accept the name and value (for Set functions) as parameters, handle the null check, and returns the value or an empty string (for Get Functions). This will make your code much easier to maintain and possibly improve, and if you decide to use something other than Cookies to store/retrieve options in the future, you'll only have to change the functions.