3

I have two cases to extract information from the IDataReader object

Case - 1 - Length, calculates ordinal and then parse string

public static string GetString(IDataReader rdr, string columnName)
{
    int ordinal = rdr.GetOrdinal(columnName);
    if (rdr.IsDBNull(ordinal))
    {
        return string.Empty;
    }
    return (string)rdr[ordinal];
}

Case - 2, short way, getting data without calculating ordinal

public static string GetString(IDataReader rdr, string columnName)
{
    return (string)rdr[columnName];
}

Which technique should be preferred and why and if any specific context ?

Pankaj
  • 9,749
  • 32
  • 139
  • 283
  • Personally I would use the second.. that is if you don't care about checking if the columnName exist or not.. are you always certain to get non null or non empty values..?? – MethodMan Feb 16 '12 at 18:55
  • What Pavel said is correct, however , your functions differ in that they return string.Empty and null which may yield unexpected results in how you use the return value. – Eric H Feb 16 '12 at 19:04
  • 1
    @EricH The second sample will throw an InvalidCastException for null data. You can't cast DBNull to string. – phoog Feb 16 '12 at 19:41
  • possible duplicate of [.NET SqlDataReader Item\[\] vs. GetString(GetOrdinal())?](http://stackoverflow.com/questions/6020117/net-sqldatareader-item-vs-getstringgetordinal) – nawfal Nov 08 '13 at 15:03

3 Answers3

4

SqlDataReader's this[string name] looks like:

public override object this[string name]
{
    get
    {
        return this.GetValue(this.GetOrdinal(name));
    }
}

so it calculates ordinal internaly, and there is no difference what way to use.

UPDATE

Yo can rewrite your code as:

public static string GetString(IDataReader rdr, string columnName)
{
    return (rdr[columnName] as String)??String.Empty;
}
Pavel Krymets
  • 6,253
  • 1
  • 22
  • 35
  • will you like to share the sample link ? – Pankaj Feb 16 '12 at 19:05
  • @Pankaj not that if you're caching `GetOrdinal` as shown here: http://stackoverflow.com/questions/6020117/net-sqldatareader-item-vs-getstringgetordinal then `reader[i]` will show marginal performance improvement. – nawfal Jul 31 '15 at 16:50
2

Here's what MSIL looks like for your first method:

.method public hidebysig static string  GetString(class [System.Data]System.Data.IDataReader rdr,
                                                  string columnName) cil managed
{
  // Code size       49 (0x31)
  .maxstack  2
  .locals init ([0] int32 ordinal,
           [1] string CS$1$0000,
           [2] bool CS$4$0001)
  IL_0000:  nop
  IL_0001:  ldarg.0
  IL_0002:  ldarg.1
  IL_0003:  callvirt   instance int32 [System.Data]System.Data.IDataRecord::GetOrdinal(string)
  IL_0008:  stloc.0
  IL_0009:  ldarg.0
  IL_000a:  ldloc.0
  IL_000b:  callvirt   instance bool [System.Data]System.Data.IDataRecord::IsDBNull(int32)
  IL_0010:  ldc.i4.0
  IL_0011:  ceq
  IL_0013:  stloc.2
  IL_0014:  ldloc.2
  IL_0015:  brtrue.s   IL_0020
  IL_0017:  nop
  IL_0018:  ldsfld     string [mscorlib]System.String::Empty
  IL_001d:  stloc.1
  IL_001e:  br.s       IL_002f
  IL_0020:  ldarg.0
  IL_0021:  ldloc.0
  IL_0022:  callvirt   instance object [System.Data]System.Data.IDataRecord::get_Item(int32)
  IL_0027:  castclass  [mscorlib]System.String
  IL_002c:  stloc.1
  IL_002d:  br.s       IL_002f
  IL_002f:  ldloc.1
  IL_0030:  ret
} // end of method Program::GetString

And for your second method:

.method public hidebysig static string  GetStringShort(class [System.Data]System.Data.IDataReader rdr,
                                                       string columnName) cil managed
{
  // Code size       18 (0x12)
  .maxstack  2
  .locals init ([0] string CS$1$0000)
  IL_0000:  nop
  IL_0001:  ldarg.0
  IL_0002:  ldarg.1
  IL_0003:  callvirt   instance object [System.Data]System.Data.IDataRecord::get_Item(string)
  IL_0008:  castclass  [mscorlib]System.String
  IL_000d:  stloc.0
  IL_000e:  br.s       IL_0010
  IL_0010:  ldloc.0
  IL_0011:  ret
} // end of method Program::GetStringShort

So the methods are definitely not the same. As to which is better, you don't say why you want to calculate the ordinal so it's hard to say which is better for your situation.

Keith Hoffman
  • 608
  • 5
  • 16
0

I don't think there's really a difference (they do the same thing), the second seems more readable to me as the introduction of the 'index' is confusing for those who don't know how the reader is working. Hide needless complexity I say. You should go a step further and make it a generic you can use anywhere, like this:

    private static T FromDbValue<T>(IDataReader rdr, string columnName)
    {
        var value = rdr[columnName];

        if (value == DBNull.Value)
        {
            return default(T);
        }

        return (T)value;
    }

Invoking it is easy:

var someString = FromDbValue<string>(rdr, "CustomerName");

Edit: You're not checking for DBNull in your second example, so the cast would fail on nulls. However, generally speaking the two approaches are the same.

Arbiter
  • 984
  • 1
  • 10
  • 24
  • The question is not in context of Generic, it's non generic. Any link you will like to share ? – Pankaj Feb 16 '12 at 18:57
  • There is none, I cut some extra code out and missed that. Removed. Thanks. – Arbiter Feb 16 '12 at 18:58
  • Please update the answer in the same context... Or you are expecting downvotes... – Pankaj Feb 16 '12 at 19:00
  • I don't know what you mean? I provided a usage example for my method. If you can't use generics, use your method. – Arbiter Feb 16 '12 at 19:04
  • This could be a two line function: var value = rdr[columnName]; return (value == DBNull.Value) ? default(T) : return (T)value; – Eric H Feb 16 '12 at 19:05
  • @EricH Yea, that works too, but I avoid ternary operators generally, as it's can be harder to read. – Arbiter Feb 16 '12 at 19:08