Better Code than This......

L

Larry Smith

If Not dtrMyDataReader.IsDBNull(dtrMyDataReader.GetOrdinal("Customer")) Then

strCustomer = dtrMyDataReader("Customer").ToString()



End If

Where "Customer" is a string data type.

Is there any better code to check for null values in the first line.

Is there any better way the code can be written?

..Net 1.1 and Visual Basic and ODBC connections.

Thank you,

Larry
 
N

neilmcguigan

1. faster to use the index and not the column name, but better practice
to use the column name, i'd say:

dim ixCustomer as integer = dr.GetOrdinal("Customer")

2. better to use typed accessor:

if not dr.IsDBNull(ixCustomer) then
strCustomer = dr.GetString(ixCustomer)
end if

HTH
 
K

Kevin Frey

Well, I'm not particularly familiar with DataReader's but a couple of things
come to mind to make your code "Better":

1. Is the query going to be the same all the time - can you gain an
efficiency perspective by either precomputing the ordinal value of the
field, or precomputing a "pointer/reference"" to the object (assuming the
reader does not recreate it each time)?

2. In the first line (IsDBNull) you go to the trouble of computing the
ordinal of the field (admittedly because IsDBNull only offers this variant),
then you just throw it away and look up the field by name in the second
line. You would gain better efficiency by just holding the ordinal as a
temporary value (eg. within the routine) and using it in both places. Given
that the sample DataReader implementations I have seen simply use a linear
search to compute an Ordinal, that's an awful lot of "lookups" happening
every time, so minimising it would be worthwhile. A dictionary lookup will
be more efficient, but still not as efficient as not doing it.

3. Since I come from a large systems background, the use of "magic
constants" such as hard-coding the "Customer" literal is pretty dodgy in my
opinion. These should be declared as constants somewhere. Having said that,
RAD-style development tools such as VS.NET that encourage you to plug in
"properties" all over the place against objects don't really encourage
constant-reuse so this will have to be something you make the call on. This
is something I'm grappling with at present.

Hope this helps.

Kevin
 
M

Mythran

Larry Smith said:
If Not dtrMyDataReader.IsDBNull(dtrMyDataReader.GetOrdinal("Customer"))
Then

strCustomer = dtrMyDataReader("Customer").ToString()



End If

Where "Customer" is a string data type.

Is there any better code to check for null values in the first line.

Is there any better way the code can be written?

.Net 1.1 and Visual Basic and ODBC connections.

Thank you,

Larry

Public Function GetString( _
ByVal Reader As DataReader, _
ByVal Column As String _
) As String
Dim idx As Integer = Reader.GetOrdinal(Column)
If Not Reader.IsDBNull(idx)
Return Reader.GetString(idx)
End If
Return String.Empty
End Function

Simple utility function to make all your if...then's a single-line ... for
the most part.

Dim customer As String = GetString(myReader, "Customer")

HTH :)

Mythran
 
L

Larry Smith

I am looking for efficient code.

This approach makes the code simpler & makes it maintainable....

Let me rephrase the question - "Better in terms of efficiency"

Thank you,

Larry
 
N

neilmcguigan

well to make it super fast, you want the code to do the least work...

a) make sure that value is never NULL in the db, then you don't have to
test for it
b) if you are just returning the customer value (and no other fields)
then make it the output or return parameter of a stored procedure, or
use ExecuteScalar
c) hardcode the ordinal constant.

but as always, code for clarity and maintainability, until performance
becomes an issue.
 
M

mszanto

First of all, if you're casting the field to a string then why bother
to check if the value is null? Whats wrong with setting strCustomer to
"" which is what you will get when you cast a dbnull to a string?

Avoid having to check for null values by replacing all null values with
an empty string when you retrieve the data from your database. For
example:

select isnull(Customer, "") as Customer from myTable.

Question: Why are you using GetOrdinal? Why not simplify it with:

If Not dtrMyDataReader.IsDBNull(Customer") then...

Also, although it doesn't hurt, there really is no reason to use
..ToString() in vb.net.

mike
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,898
Latest member
BlairH7607

Latest Threads

Top