I would like to prevent further processing on an object if it is null.
In the following code I check if the object is null by either:
if (!data.Equals(null))
and
if (data != null)
However, I receive a NullReferenceException
at dataList.Add(data)
. If the object was null, it should never have even entered the if
-statement!
Thus, I'm asking if this is proper way of checking if an object is null:
public List<Object> dataList;
public bool AddData(ref Object data)
bool success = false;
try
{
// I've also used "if (data != null)" which hasn't worked either
if (!data.Equals(null))
{
//NullReferenceException occurs here ...
dataList.Add(data);
success = doOtherStuff(data);
}
}
catch (Exception e)
{
throw new Exception(e.ToString());
}
return success;
}
If this is the proper way of checking if the object is null, what am I doing wrong (how can I prevent further processing on the object to avoid the NullReferenceException)?
use:
Conditional use:
Update (another way) updated 08/31/2017. Thanks for the comment.
No, you should be using
!=
. Ifdata
is actually null then your program will just crash with aNullReferenceException
as a result of attempting to call theEquals
method onnull
. Also realize that, if you specifically want to check for reference equality, you should use theObject.ReferenceEquals
method as you never know howEquals
has been implemented.Your program is crashing because
dataList
is null as you never initialize it.In addition to @Jose Ortega answer, its better for use extension method
And use
IsNull
method for all of object like:It's not
data
that isnull
, butdataList
.You need to create one with
Even better: since it's a field, make it
private
. And if there's nothing preventing you, make it alsoreadonly
. Just good practice.Aside
The correct way to check for nullity is
if(data != null)
. This kind of check is ubiquitous for reference types; evenNullable<T>
overrides the equality operator to be a more convenient way of expressingnullable.HasValue
when checking for nullity.If you do
if(!data.Equals(null))
then you will get aNullReferenceException
ifdata == null
. Which is kind of comical since avoiding this exception was the goal in the first place.You are also doing this:
This is definitely not good. I can imagine that you put it there just so you can break into the debugger while still inside the method, in which case ignore this paragraph. Otherwise, don't catch exceptions for nothing. And if you do, rethrow them using just
throw;
.Jeffrey L Whitledge is right. Your `dataList´-Object itself is null.
There is also another problem with your code: You are using the ref-keyword, which means the argument data cannot be null! The MSDN says:
It's also not a good idea to use generics with the type `Object´. Generics should avoid boxing/unboxing and also ensure type safety. If you want a common type make your method generic. Finally your code should look like this:
As others have already pointed out, it's not
data
but rather likelydataList
that isnull
. In addition to that...catch
-throw
is an antipattern that almost always makes me want to throw up every time that I see it. Imagine that something goes wrong deep in something thatdoOtherStuff()
calls. All you get back is anException
object, thrown at thethrow
inAddData()
. No stack trace, no call information, no state, nothing at all to indicate the real source of the problem, unless you go in and switch your debugger to break on exception thrown rather than exception unhandled. If you are catching an exception and just re-throwing it in any way, particularly if the code in the try block is in any way nontrivial, do yourself (and your colleagues, present and future) a favor and throw out the entiretry
-catch
block. Granted,throw;
is better than the alternatives, but you are still giving yourself (or whoever else is trying to fix a bug in the code) completely unnecessary headaches. This is not to say that try-catch-throw is necessarily evil per se, as long as you do something relevant with the exception object that was thrown inside the catch block.Then there's the potential problems of catching
Exception
in the first place, but that's another matter, particularly since in this particular case you throw an exception.Another thing that strikes me as more than a little dangerous is that
data
could potentially change value during the execution of the function, since you are passing by reference. So the null check might pass but before the code gets to doing anything with the value, it's changed - perhaps tonull
. I'm not positive if this is a concern or not (it might not be), but it seems worth watching out for.