Is this script ok?

[rjoseph]

Hi Guys

I am a novice programmer and have basicaly learned about stored procedure overnight. I have just created the following script on my page to call and execture one of my stored proedures and although it works fine, I am not sure if it is ok. If someone could take a look at my script and let me know if it's ok then that would be great.

Best regards

Rod from the UK

---------------------------

<%
Dim Currpage, pageLen, lastNumber, PageRem, PageTen
Dim connection, recordset, sSQL, sConnString, next10, prev10, P
Dim RSPrevPage, RSNextPage, start 

If IsEmpty(Request.Querystring("PageNo")) then
CurrPage = 1
Else
CurrPage = Cint(Replace(Replace(Replace(Replace(Replace(Replace(Replace(Replace(Replace(Request("PageNo"),"'",""), ";", "" ), "-", " " ), "(", "" ), ")", "" ), """", ""), "drop", ""), "update", ""), "delete", ""))
End If 

'the two functions below return the next 10 and prev 10 page number
Function getNext10(num)
pageLen = len(num)
If pageLen = 1 Then
next10 = 10
Else If pageLen>1 Then
pageRem = 10
pageTen = right(num, 1)
next10 = num + pageRem - pageTen
End If
End If
getNext10 = next10
End Function

Function getPrev10(num)
pageLen = len(num)
If pageLen = 1 then
prev10 = 1
Else If pageLen>1 then
lastNumber = right(num, 1)
prev10 = num - lastNumber - 10
End If
End If
If prev10 = 0 then
prev10 = 1
End If
getPrev10 = prev10
End Function
%>

<%
function getAdverts(postcode, distance)
  set cmd = server.CreateObject("ADODB.command")
  set cmd.ActiveConnection = oDBConn

  cmd.CommandType = 4 'This is a stored procedure command
  cmd.CommandText = "sp_mystoredprocedure"
  
  cmd.Parameters(1) = "" & make & ""
  cmd.Parameters(2) = "" & model & ""
  cmd.Parameters(3) = "" & bodystyle & ""
  cmd.Parameters(4) = "" & engine & ""
  cmd.Parameters(5) = "" & regyear & ""
  cmd.Parameters(6) = "" & colour & ""
  cmd.Parameters(7) = "" & transmission & ""
  cmd.Parameters(8) = "" & fuel & ""
  cmd.Parameters(9) = "" & county & ""
  cmd.Parameters(10) = "" & orderby & ""


  set oRS = Server.CreateObject("ADODB.recordset")
  with oRS
    .CursorLocation = 3
    
    .open cmd
  end with
  
  set getAdverts = oRS
end function

set RecordSet = getAdverts("", "")

Recordset.PageSize = 10

'get the next 10 and prev 10 page number
next10 = getNext10(CurrPage)
prev10 = getPrev10(CurrPage)

'If there are no records
If Recordset.EOF Then
%>


THERE ARE NO RESULTS

<%

Else

Recordset.AbsolutePage = CurrPage

  dim c
  c = 0
  do Until Recordset.AbsolutePage <> CurrPage OR Recordset.Eof


make = Recordset ("make")
model = Recordset ("model")
body = Recordset ("body")
engine = Recordset ("engine")
regyear = Recordset ("regyear")
colour = Recordset ("colour")
transmission = Recordset ("transmission")
fuel = Recordset ("fuel")
county = Recordset ("county")
%>


<hr>
<%=make%><br>
<%=model%><br>
<%=body%><br>
<%=engine%><br>
<%=regyear%><br>
<%=colour%><br>
<%=transmission%><br>
<%=fuel%><br>
<%=county%>

<hr>

<%
    RecordSet.moveNext
  loop
%>

<%
End If

'the next 2 lines setup the page number for the "previous" and "next" links
RSPrevPage = CurrPage -1
RSNextPage = CurrPage + 1

'find out the number of pages returned in the recordset
'if the Next10 page number is greater than the recordset page count
'then set Next10 to the recordset pagecount
If Next10 > Recordset.PageCount Then
Next10 = Recordset.PageCount
End If

'the variable start determines where to start the page number navigation
' i.e. 1, 10, 20, 30 and so on. 
If prev10 = 1 AND next10 - 1 < 10 Then
start = 1
Else
start = Next10 - 10
If right(start, 1) > 0 Then
start = replace(start, right(start, 1), "0")
start = start + 10
End If
End If

%>


<%
pageno=Replace(Replace(Replace(Replace(Replace(Replace(Replace(Replace(Replace(request.querystring("pageno"),"'",""), ";", "" ), "-", " " ), "(", "" ), ")", "" ), """", ""), "drop", ""), "update", ""), "delete", "")
if request.querystring("pageno") = "" Then
pageno=1
else
pageno=Replace(Replace(Replace(Replace(Replace(Replace(Replace(Replace(Replace(request.querystring("pageno"),"'",""), ";", "" ), "-", " " ), "(", "" ), ")", "" ), """", ""), "drop", ""), "update", ""), "delete", "")
end if

response.write("<table border=0 cellpadding=0 cellspacing=5 bgcolor=#D6D6D6 height=35 width=670><tr><td width=130 bgcolor=#808080><p align=center><font color=#FFFFFF size=2><b>Page " & Pageno & " </b>of<b> " & recordset.pagecount & "</b></font></td><td width=540><div align=center>")

'This checks to make sure that there is more than one page of results
If Recordset.PageCount > 1 Then
'Work out whether to show the Previous 10 '<<' 
If currpage > 1 Then
response.write("<font size=2 face=arial color=#000000><a class=type5 href=""q.asp?car=" & car & "&orderby=" & orderby & "&PageNo=" & Prev10 & """><u>-10</u></a></font> ")
End If
'Work out whether to show the Previous link '<' 
If NOT RSPrevPage = 0 then
response.write("<font size=3 face=arial color=#000000> | <a class=type5 href=""q.asp?car=" & car & "&orderby=" & orderby & "&PageNo=" & RSPrevPage & """><u>‹‹Prev page</u></a></font> ")
End If

'Loop through the page number navigation using P as our loopcounter variable 
For P = start to Next10

If NOT P = CurrPage then
response.write("<font size=3 face=arial color=#000000> | <a class=type5 href=""q.asp?car=" & car & "&orderby=" & orderby & "&PageNo=" & P & """><u>" & P & "</u></a></font> ")
Else
'Don't hyperlink the current page number 
response.write(" <font size=3 face=arial color=#000000> | <b>" & P & " </b></font>")
End If
Next
'this does the same as the "previous" link, but for the "next" link
If NOT RSNextPage > Recordset.PageCount Then
response.write("<font size=3 face=arial color=#000000> | <a class=type5 href=""q.asp?car=" & car & "&orderby=" & orderby & "&PageNo=" & RSNextPage & """><u>Next page››</u></a></font> ")
End If

'Work out whether to show the Next 10 '>>' 
If NOT Next10 = Recordset.PageCount Then
response.write("<font size=2 face=arial color=#000000>  | <a class=type5 href=""q.asp?car=" & car & "&orderby=" & orderby & "&PageNo=" & Next10 & """><u>10+</u></a></font>")
End If
End If

response.write("</div></td></tr></table><br>")

set cmd = nothing
Recordset.close 
set Recordset = nothing
%>

---------------------------

[jhardman]

Wow, when I was a novice I wasn't nearly that careful! I think you covered all the bases, did everything I would have suggested to be safe.

Jared

[rjoseph]

All those long frustrating nights are paying off!

Thanks Jared