VB COM Critique

[kreplech]

I am fairly new to this and I would love some criticism (constructive, of course) regarding the code below...


option Explicit

private objConn as ADODB.Connection
private objCmd as ADODB.Command
private objRs as ADODB.Recordset

public Sub OnStartPage(byref rScriptingcontext as ScriptingContext)
set gAspAppl = rScriptingcontext.Application
set gAspReq = rScriptingcontext.Request
set gAspResp = rScriptingcontext.Response
set gAspServer = rScriptingcontext.Server
set gAspSession = rScriptingcontext.Session
End Sub

public Sub OnEndPage()
set gAspAppl = nothing
set gAspReq = nothing
set gAspResp = nothing
set gAspServer = nothing
set gAspSession = nothing
End Sub

public Sub propertyFinder(SQL, database)

Dim style as string, cityCounty as string, city as string, county as string, zipCode as string, bed as string, bath as string, anyprice as string, min, max as string
Dim pool as string, water as string, garage as string, security as string, adult as string, acreage as string
Dim strConn as string, sqlString as string
Dim count as Integer

set objConn = new ADODB.Connection
set objCmd = new ADODB.Command
set objRs = new ADODB.Recordset

strConn = database
sqlString = SQL

With objConn
.ConnectionTimeout = 30
.CommandTimeout = 30
.Open (database)
End With

style = gAspReq.Form("style")
cityCounty = gAspReq.Form("cityCounty")
city = gAspReq.Form("city")
county = gAspReq.Form("county")
zipCode = gAspReq.Form("zipCode")
bed = gAspReq.Form("bed")
bath = gAspReq.Form("bath")
anyprice = gAspReq.Form("anyprice")
min = gAspReq.Form("min")
max = gAspReq.Form("max")
pool = gAspReq.Form("pool")
water = gAspReq.Form("water")
garage = gAspReq.Form("garage")
security = gAspReq.Form("security")
adult = gAspReq.Form("adult")
acreage = gAspReq.Form("acreage")


objRs.Open sqlString, objConn, adOpenDynamic
With gAspResp
Do While Not objRs.EOF
count = count + 1
.Write "
<p>" & count & ".<strong>  property Type:</strong> " & objRs(2)
.Write "      <strong>Listing date:</strong> " & objRs(3)
.Write "      <strong>Address:</strong> " & objRs(5) & " " & objRs(6) & " " & objRs(7) & "   " & objRs(8)
.Write "      <strong>Spec ID:</strong> " & objRs("T28PropertySpecsID") & "</p>"
objRs.MoveNext
Loop
End With
If (count = 0) then
gAspResp.Write "Your search returned <strong>0</strong> results."
End If
gAspResp.Write "<p>" & sqlString & "</p>"
objRs.Close
set objRs.ActiveConnection = nothing

End Sub





Web Dev
Boca Raton, FL
kreplech@aol.com

[Cakkie]

First of all, this is a great example of how to process nd ASP page with a VB dll. Finally someone who realizes that this is way faster than letting the ASP page process the requests. However there are some things that might need some improvement (it may look a lot, but it's there only to help)

Fisrst of all, you declare a whole lot of variables, assign a value to it, but don't use them. If you don't need them, don't declare and assign them. This is only a waste of memory and performance.

The same goes for all the scriptingcontext objects. You only use 2, so only declare and assign 2.

For the recordset, I would set the cursorlocation to client, and set the cursortype to a static type. You are using dynamic, but aren't changing anything, and are only moving forward. A statis, forward-only cursortype can really speed up things.

Another thing is that you set the activeconnection property to nothing at the end of your code. Set the activeconnection property to nothing after you have opened the recordset, after wich the connection itself (objConn) can be set to nothing.

A final thing I would like to note is that when addressing fields of a recordset, you should rather use the name insted of the index. For some reasons, the index may change (like adding a field to the underlying table or select statement). Also, using names improves readability.

For the rest I can say that this is a good formed, easy to read peace of code. I think a lot of people could take a example from you.
Keep it up.

Tom Cannaerts
slisse@planetinternet.be

Programming today is a race between software engineers striving to build bigger and better idot-proof programs, and the universe trying to produce bigger and better idiots. So far, the universe is winning -- Rich Cook

[Simon Pallister]

A couple of comments:

1.Use of the ScriptingContext object is outdated and ObjectContext should be used instead.

2. No naming convention for variable names - use hungarian or similar.

3. No error handling - some kind of exception hadling should be used as a minimum

[dmeikle]

wow, really like what you had to say about the critique. Nice to finally hear someone talking about server resources. Many web programmers seem to never realize the only difference between a desktop computer and 'some' web servers is maybe more ram, depending on the system. that taken into account, we're not runnign an app on a standalone system, it's run with mebbe a few hundred people logged on at the same time, all demading cpu cycles. People should always consider memory and cpu usage, not just write software 'that works'....

on another note, I would forsake readbility for usage of the rs index values. It doesn't need to cross reference the column name to the rs index, less work. might take a jog in the coding to make a query change, but a few minutes at design time boosts performance at run time.


BTW, are you using MTS? I have a problem where if no user is logged in, the dll's won't run..lol
no one I know can find a way around this...any suggestions?

[Cakkie]

Hmm, I disagree on the latter. Error handling should be done by whatever called the dll. This is very important if the dll is used on several places, where sometimes you just want to do other stuff than the default error handling.

Tom Cannaerts
slisse@planetinternet.be

Programming today is a race between software engineers striving to build bigger and better idot-proof programs, and the universe trying to produce bigger and better idiots. So far, the universe is winning -- Rich Cook