tags:

views:

7191

answers:

13

This question comes up occasionally but I haven't seen a satisfactory answer.

A typical pattern is (row is a DataRow):

 if (row["value"] != DBNull.Value)
 {
      someObject.Member = row["value"];
 }

My first question is which is more efficient (I've flipped the condition):

  row["value"] == DBNull.Value; // Or
  row["value"] is DBNull; // Or
  row["value"].GetType() == typeof(DBNull) // Or... any suggestions?

This indicates that .GetType() should be faster, but maybe the compiler knows a few tricks I don't?

Second question, is it worth caching the value of row["value"] or does the compiler optimize the indexer away anyway?

eg.

  object valueHolder;
  if (DBNull.Value == (valueHolder = row["value"])) {}

Disclaimers:

  1. row["value"] exists.
  2. I don't know the column index of the column (hence the column name lookup)
  3. I'm asking specifically about checking for DBNull and then assignment (not about premature optimization etc).

Edit:

I benchmarked a few scenarios (time in seconds, 10000000 trials):

row["value"] == DBNull.Value: 00:00:01.5478995
row["value"] is DBNull: 00:00:01.6306578
row["value"].GetType() == typeof(DBNull): 00:00:02.0138757

Object.ReferenceEquals has the same performance as "=="

The most interesting result? If you mismatch the name of the column by case (eg. "Value" instead of "value", it takes roughly ten times longer (for a string):

row["Value"] == DBNull.Value: 00:00:12.2792374

The moral of the story seems to be that if you can't look up a column by it's index, then ensure that the column name you feed to the indexer matches the DataColumn's name exactly.

Caching the value also appears to be nearly twice as fast:

No Caching: 00:00:03.0996622
With Caching: 00:00:01.5659920

So the most efficient method seems to be:

 object temp;
 string variable;
 if (DBNull.Value != (temp = row["value"])
 {
      variable = temp.ToString();
 }

This was a good learning experience.

+1  A: 

You should use the method:

Convert.IsDBNull()

Considering it's built-in to the Framework, I would expect this to be the most efficient.

I'd suggest something along the lines of:

int? myValue = (Convert.IsDBNull(row["column"]) ? null : (int?) Convert.ToInt32(row["column"]));

And yes, the compiler should cache it for you.

Jon Grant
Well, *all* the options mentioned are built into the framework... Actually, Convert.IsDBNull does a lot of extra work relating to IConvertible...
Marc Gravell
And re the cache - if you mean with the conditional example, no - it really shouldn't (and doesn't). It will execute the indexer twice.
Marc Gravell
Oh, and that code doesn't compile - but add an (int?) to one of them, and you'll see (in the IL) 2 of: callvirt instance object [System.Data]System.Data.DataRow::get_Item(string)
Marc Gravell
+5  A: 

The compiler won't optimise away the indexer (i.e. if you use row["value"] twice), so yes it is slightly quicker to do:

object value = row["value"];

and then use value twice; using .GetType() risks issues if it is null...

DBNull.Value is actually a singleton, so to add a 4th option - you could perhaps use ReferenceEquals - but in reality, I think you're worrying too much here... I don't think the speed different between "is", "==" etc is going to be the cause of any performance problem you are seeing. Profile your entire code and focus on something that matters... it won't be this.

Marc Gravell
In virtually all cases == is going to be equivalent to ReferenceEquals (esp. to DBNull) and it's much more readable. Use @Marc Gravell's optimization if you want, but I'm with him -- probably not going to help much. BTW, reference equality ought to always beat type checking.
tvanfosson
A: 

I always use :

if (row["value"] != DBNull.Value)
  someObject.Member = row["value"];

Found it short and comprehensive.

Daok
+3  A: 

I personally favour this syntax, which uses the explicit IsDbNull method exposed by IDataRecord, and caches the column index to avoid a duplicate string lookup.

Expanded for readability, it goes something like:

int columnIndex = row.GetOrdinal("Foo");
string foo; // the variable we're assigning based on the column value.
if (row.IsDBNull(columnIndex)) {
  foo = String.Empty; // or whatever
} else { 
  foo = row.GetString(columnIndex);
}

Rewritten to fit on a single line for compactness in DAL code - note that in this example we're assigning int bar = -1 if row["Bar"] is null.

int i; // can be reused for every field.
string foo  = (row.IsDBNull(i  = row.GetOrdinal("Foo")) ? null : row.GetString(i));
int bar = (row.IsDbNull(i = row.GetOrdinal("Bar")) ? -1 : row.GetInt32(i));

The inline assignment can be confusing if you don't know it's there, but it keeps the entire operation on one line, which I think enhances readability when you're populating properties from multiple columns in one block of code.

Dylan Beattie
DataRow doesn't implement IDataRecord though.
ilitirit
A: 

I've been neurotic about always checking like this:

someObject.Member =  ( DBNull.Value.Equals(row["value"]) ? null : row["value"]);
or 
if (! DBNull.Value.Equals(row["value"]) someObject.Member = row["value"];


or sometimes
    someObject.Member =  ( DBNull.Value.Equals(row["value"]) ? someObject.Member : row["value"]);

If these methods of doing things aren't right, I'd definately want to know, since this is the way I consistantly handle variable assignments when checking for DBNull.

stephenbayer
You are getting the value from the row twice which will cause it to be inefficient.
Stevo3000
A: 

Not that I've done this, but you could get around the double indexer call and still keep your code clean by using a static / extension method.

Ie.

public static IsDBNull<T>(this object value, T default)
{
    return (value == DBNull.Value)
        ? default
        : (T)value;
}

public static IsDBNull<T>(this object value)
{
    return value.IsDBNull(default(T));
}

Then:

IDataRecord record; // Comes from somewhere

entity.StringProperty = record["StringProperty"].IsDBNull<string>(null);
entity.Int32Property = record["Int32Property"].IsDBNull<int>(50);

entity.NoDefaultString = record["NoDefaultString"].IsDBNull<string>();
entity.NoDefaultInt = record["NoDefaultInt"].IsDBNull<int>();

Also has the benefit of keeping the null checking logic in one place. Downside is, of course, that it's an extra method call.

Just a thought.

Richard Szalay
Adding an extension method on object is very broad, though. Personally I might have considered an extension method on DataRow, but not object.
Marc Gravell
True, though keep in mind that extension methods are only available when the extension class's namespace is imported.
Richard Szalay
A: 

This is how I handle reading from DataRows

///<summary>
/// Handles operations for Enumerations
///</summary>
public static class DataRowUserExtensions
{
    /// <summary>
    /// Gets the specified data row.
    /// </summary>
    /// <typeparam name="T"></typeparam>
    /// <param name="dataRow">The data row.</param>
    /// <param name="key">The key.</param>
    /// <returns></returns>
    public static T Get<T>(this DataRow dataRow, string key)
    {
        return (T) ChangeTypeTo<T>(dataRow[key]);
    }

    private static object ChangeTypeTo<T>(this object value)
    {
        Type underlyingType = typeof (T);
        if (underlyingType == null)
            throw new ArgumentNullException("value");

        if (underlyingType.IsGenericType && underlyingType.GetGenericTypeDefinition().Equals(typeof (Nullable<>)))
        {
            if (value == null)
                return null;
            var converter = new NullableConverter(underlyingType);
            underlyingType = converter.UnderlyingType;
        }

        // Try changing to Guid  
        if (underlyingType == typeof (Guid))
        {
            try
            {
                return new Guid(value.ToString());
            }
            catch

            {
                return null;
            }
        }
        return Convert.ChangeType(value, underlyingType);
    }
}

Usage example:

if (dbRow.Get<int>("Type") == 1)
{
    newNode = new TreeViewNode
                  {
                      ToolTip = dbRow.Get<string>("Name"),
                      Text = (dbRow.Get<string>("Name").Length > 25 ? dbRow.Get<string>("Name").Substring(0, 25) + "..." : dbRow.Get<string>("Name")),
                      ImageUrl = "file.gif",
                      ID = dbRow.Get<string>("ReportPath"),
                      Value = dbRow.Get<string>("ReportDescription").Replace("'", "\'"),
                      NavigateUrl = ("?ReportType=" + dbRow.Get<string>("ReportPath"))
                  };
}

Props to Monsters Got My .Net for ChageTypeTo code.

Chris Marisic
A: 

i've done something similar with extension methods. here's my code

public static class DataExtensions
{
    /// <summary>
    /// Gets the value.
    /// </summary>
    /// <typeparam name="T">The type of the data stored in the record</typeparam>
    /// <param name="record">The record.</param>
    /// <param name="columnName">Name of the column.</param>
    /// <returns></returns>
    public static T GetColumnValue<T>(this IDataRecord record, string columnName)
    {
        return GetColumnValue<T>(record, columnName, default(T));
    }

    /// <summary>
    /// Gets the value.
    /// </summary>
    /// <typeparam name="T">The type of the data stored in the record</typeparam>
    /// <param name="record">The record.</param>
    /// <param name="columnName">Name of the column.</param>
    /// <param name="defaultValue">The value to return if the column contains a <value>DBNull.Value</value> value.</param>
    /// <returns></returns>
    public static T GetColumnValue<T>(this IDataRecord record, string columnName, T defaultValue)
    {
        object value = record[columnName];
        if (value == null || value == DBNull.Value)
        {
            return defaultValue;
        }
        else
        {
            return (T)value;
        }
    }
}

to use, you would do something like

int number = record.GetColumnValue<int>("Number",0)
Darren Kopp
A: 

I try to avoid this check as much as possible.

Obviously doesn't need to be done for columns that can't hold null.

If you're storing in a Nullable value type (int?, etc.), you can just convert using as int?.

If you don't need to differentiate between string.Empty and null, you can just call .ToString(), since DBNull will return string.Empty.

bdukes
A: 

I would use the following code in C#, vb.net is not as simple.

The code assigns the value if it is not null/dbnull otherwise it asigns the default which could be set to the LHS value allowing the compiler to ignore the assign.

oSomeObject.IntMemeber = oRow["Value"] as int? ?? iDefault;
oSomeObject.StringMember = oRow["Name"] as string ?? sDefault;
Stevo3000
The VB.NET version *is* as simple: `oSomeObject.IntMember = If(TryCast(oRow("Value), Integer?), iDefault)`.
Dan Tao
@Dan Tao - I don't think you've compiled that code. Look at an old question of mine which explains why your code will not work. http://stackoverflow.com/questions/746767/how-to-acheive-the-c-as-keyword-for-value-types-in-vb-net
Stevo3000
@Stevo3000: And once again, commenting on an SO question while away from my own computer (with dev tools on it) has proven to be a mistake! You are right; I'm surprised to learn that `TryCast` doesn't provide the same convenient functionality as C#'s `as` operator for `Nullable(Of T)` types. The closest way I can think of to mimic this is to write your own function, as I've now suggested in my answer.
Dan Tao
A: 

I have IsDBNull in program which reads a lot of data from DB. With IsDBNull it loads data with: ~20 sec Without IsDBNull: ~ 1 sec

So i think better use

public String TryGetString(SqlDataReader sqlReader, int row)
    {
        String res = "";
        try
        {
            res = sqlReader.GetString(row);
        }
        catch (Exception)
        { }
        return res;
    }
Mastahh
A: 

There is the troublesome case where the object could be a string. The below extension method code handles all cases. Here's how you would use it:

    static void Main(string[] args)
    {
        object number = DBNull.Value;

        int newNumber = number.SafeDBNull<int>();

        Console.WriteLine(newNumber);
    }



    public static T SafeDBNull<T>(this object value, T defaultValue) 
    {
        if (value == null)
            return default(T);

        if (value is string)
            return (T) Convert.ChangeType(value, typeof(T));

        return (value == DBNull.Value) ? defaultValue : (T)value;
    } 

    public static T SafeDBNull<T>(this object value) 
    { 
        return value.SafeDBNull(default(T)); 
    } 
Saleh Najar
+2  A: 

I must be missing something. Isn't checking for DBNull exactly what the DataRow.IsNull method does?

I've been using the following two extension methods:

public static T? GetValue<T>(this DataRow row, string columnName) where T : struct
{
    if (row.IsNull(columnName))
        return null;

    return row[columnName] as T?;
}

public static string GetText(this DataRow row, string columnName)
{
    if (row.IsNull(columnName))
        return string.Empty;

    return row[columnName] as string ?? string.Empty;
}

Usage:

int? id = row.GetValue<int>("Id");
string name = row.GetText("Name");
double? price = row.GetValue<double>("Price");

If you didn't want Nullable<T> return values for GetValue<T>, you could easily return default(T) or some other option instead.


On an unrelated note, here's a VB.NET alternative to Stevo3000's suggestion:

oSomeObject.IntMember = If(TryConvert(Of Integer)(oRow("Value")), iDefault)
oSomeObject.StringMember = If(TryCast(oRow("Name"), String), sDefault)

Function TryConvert(Of T As Structure)(ByVal obj As Object) As T?
    If TypeOf obj Is T Then
        Return New T?(DirectCast(obj, T))
    Else
        Return Nothing
    End If
End Function
Dan Tao