2

The following extension method was set up in some code I maintain.

public static Int32 GetInt32(this System.Data.Common.DbDataReader reader, string name)
{
   return reader.GetInt32(reader.GetOrdinal(name));
}

Which is then called like this:

int field1 = dr.GetInt32("field1");

Is this really any better than just casting to an int like this:

int field1 = (int)dr["DiskGroupNum"];
7

There is a fundamental difference here. In one case you say, "I'm going to just assume this is an int", and in the former, you ask the reader to give you an int. If the value is already an int, I would not expect any difficulty. However, what happens when the value is a Int64? You're casting it to an int which is only 4 bytes long. You may get unexpected surprises this way if you're lucky. If you're not, the program continues like nothing happened, and you wind up with a value missing its upper 4 bytes.

However if you ask the reader to give you an Int32, you will see it complain should there be some problems with doing so. It might seem contrary writing code to potentially create exceptions, but it works in your favor ultimately, trust me. Even if something does go wrong when you don't expect it, the error is clear and can be fixed easily.

If you have a value that doesn't reflect what is read, it may cause problems elsewhere in your program and leave you scratching your head as to why.

  • 3
    +1 for delegating error/bounds checking onto the layer responsible for marshaling data back and forth. – user22815 Oct 31 '14 at 17:52
  • What does this have to do with luck? If you try to cast a boxed Int64 to Int32, you're always going to get an exception. – svick Nov 4 '14 at 22:24
  • @svick This is a for instance. We could just as easily be talking about strings using incompatible encodings or a date value that could be interpretted as an integer (milliseconds since Jan. 1, 1970). It is generally a good idea not to take risks, don't you agree? – Neil Nov 5 '14 at 9:27
1

Actually, I would go with a third option. Retrieve the column ordinal up-front and use the ordinal overloads. In any case where you have multiple rows, it saves work:

var ord_field1 = dr.GetOrdinal("field1");

while(dr.Read())
{
    ...
    var field1 = dr.GetInt32(ord_field1);
    ...
}
  • What is the benefit of this approach, besides "saving work?" – user22815 Oct 31 '14 at 17:52
  • @Snowman you get the type-checking benefits of the GetInt32 call without the extra hit of calling GetOrdinal columns * rows times – Dan Lyons Oct 31 '14 at 17:54
0

No, it is not better than casting to int (in my opinion).

In your specific example, dr["DiskGroupNum"] returns type object, which means that your second example:

int field1 = (int)dr["DiskGroupNum"];

You are actually unboxing the object to type int, which will give you an InvalidCastException if it's not exactly of type int (Int32 specifically).

Why I think it is not beneficial to have an extension method to do this is:

  1. It's more writing
  2. It's more code to debug
  3. You'll have to write specific code for different types

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.