| Author |
 Topic  |
|
seahorse
Senior Member
USA
1075 Posts |
|
|
ruirib
Snitz Forums Admin
Portugal
26364 Posts |
|
|
Nikkol
Forum Moderator
USA
6907 Posts |
 Posted - 05 September 2003 : 06:46:34
|
not sure about that ... but regardless, you've got some strange code. I'd rethink the logic behind it. For example,
line 69 write an open table tag
line 71 test for null url ... if null, response.end
line 100 (write a close table tag) is never reached!! |
Nikkol ~ Help Us Help You | ReadMe | 3.4.03 fixes | security fixes ~ |
 |
|
|
Nikkol
Forum Moderator
USA
6907 Posts |
Posted - 05 September 2003 : 06:48:59
|
perhaps this line:
Do while intRec <= objRS.PageSize
needs to be:
Do while intRec <= objRS.PageSize - 1 scratch that ... just a sec...
Do while intRec <= objRS.PageSize and not objRS.eof
(maybe  ) |
Nikkol ~ Help Us Help You | ReadMe | 3.4.03 fixes | security fixes ~ |
Edited by - Nikkol on 05 September 2003 06:53:10 |
|
|
|
ruirib
Snitz Forums Admin
Portugal
26364 Posts |
Posted - 05 September 2003 : 08:09:40
|
quote:
Originally posted by Nikkol
not sure about that ...
Hmmm... why not? I can think only of a single circunstance where the both tests with result in true (an empty file) and there are situations where both would occur independently... and those are not covered by the test.
I would probably skip the BOF test, don't think it's important. I don't remember a situation where I've used it in my own site.
I do agree however with testing for EOF in the Do While loop. |
Snitz 3.4 Readme | Like the support? Support Snitz too |
|
|
|
seahorse
Senior Member
USA
1075 Posts |
Posted - 05 September 2003 : 10:59:16
|
| Thanks for the suggestions, I'll try each in turn. I'm open to anything at the moment. |
Ken
===============
Worldwide Partner Group
Microsoft |
|
|
|
davemaxwell
Access 2000 Support Moderator
USA
3020 Posts |
Posted - 05 September 2003 : 12:15:51
|
OK, I hope you don't get offended by this....
I tried to find the error just in your code, but I just couldn't stand it. You were working WAY too hard in most places, and in a couple other places I'm surprised you weren't erroring out hard or ending up in an infinite loop. So I rewrote the code, eliminating unneeded sections of code. I tried to keep your same style of coding, but eliminated redundant code or things that were REALLY inefficient. I tried to comment why I did something, but let me know if there are any questions on why I did something.
<%@ language=vbscript %>
<!--#include file='includes/adovbs.asp' -->
<!--#include file='includes/dbConn2.asp' -->
<!--#include file='includes/functions.asp' -->
<html>
<head>
</head>
<body>
<%
' moved all declarations to one spot....
Dim SQL
Dim row_counter, intRec
Dim abspage, pagecnt, reccount, pagesize, pagenum, scriptname
' using variables is more efficient. assign values to those we can
pagesize = 50
cachesize = 50
pagenum = Trim(Request.QueryString("pagenum"))
if Len(pagenum) > 0 then
pagenum = Clng(pagenum)
else
pagenum = 1
end if
scriptname = Request.ServerVariables("SCRIPT_NAME") & "?pagenum="
' Only select the fields you need.....
SQL = "SELECT link_url, link_text, link_date "
SQL = SQL & "FROM tblLINK "
SQL = SQL & "WHERE link_featured = 'n' "
SQL = SQL & "AND delete_flag = '0' "
SQL = SQL & "ORDER BY link_date DESC"
Set objRS = Server.CreateObject("ADODB.Recordset")
objRS.PageSize = pagesize
objRS.CacheSize = cachesize
objRS.CursorLocation = adUseClient
objRS.Open SQL, strConnect, adOpenForwardOnly, adLockReadOnly
<%
' Should be the first thing you check...
if objRS.EOF AND objRS.BOF then
Response.Write "No records found!"
Response.End
else
' now get the page count and figure out what page to be on....
pagecnt = objRS.PageCount
If pagenum <= pagecnt Then
objRS.AbsolutePage = pagenum
Else
objRS.AbsolutePage = 1
pagenum = 1
End If
abspage = pagenum
' NOW loop through the records....
row_counter = 0
Response.Write "<table>"
Do while (intRec <= pagesize) and NOT objRS.EOF
if isnull(objRS("link_url")) then
' Don't think you REALLY want to do a response.end here. Do you want to end the code totally or just
' not write a table row?
Response.End
' If ending totally, should just have this instead
intRec = objRS.PageSize might be better
else
' Only need row_counter to determine row color.....
If row_counter = 0 then
Response.Write"<tr bgcolor='palegoldenrod'>" & vbNewLine
row_counter = row_counter + 1 ' Only needs done once to eliminate the possiblity
else
Response.Write"<tr bgcolor='#ffffff'>" & vbNewLine
end if
Response.Write "<td width='435'><a href='" & objRS("link_url") & "'>"
Response.Write objRS("link_text") & "</a><br /><span style='font-size: 9pt;'>" & objRS("link_date") & vbNewLine
Response.Write "</span></td>" & vbNewLine
Response.Write "</tr>" & vbNewLine
end if
intRec = intRec + 1
objRS.MoveNext
loop
Response.Write "</table>" & vbNewLine
' Used variables here.....
Response.Write "<table border='0' cellpadding='5' cellspacing='0'>" & vbNewLine
Response.Write "<tr><td>" & vbNewLine
Response.Write "<p>PageCount: " & pagecnt & " " & vbNewLine
Response.Write "Current Page: " & abspage & " " & vbNewLine
Response.Write "Total Records: " & reccount & " " & vbNewLine
Response.Write "<br>" & vbNewLine
Response.Write "<div align=""center"">" & vbNewLine
' Don't need a first page link if on first page....
If abspage = 1 Then
Response.Write "<span style=""color:silver;"">First Page</span>"
Response.Write " | "
Response.Write "<span style=""color:silver;"">Previous Page</span>"
Else
Response.Write "<a href=""" & scriptname & "1""><b>First Page</b></a>"
Response.Write " | "
Response.Write "<a href=""" scriptname & abspage - 1 & """><b>Previous Page</b></a>"
End If
Response.Write " | "
' Don't need a last page link if on last page....
If abspage < pagecnt Then
Response.Write "<a href=""" & scriptname & abspage + 1 & """>Next Page</a>"
Response.Write " | "
Response.Write "<a href=""" & scriptname & pagecnt & """><b>Last Page</b></a>"
Else
Response.Write "<span style=""color:silver;""><b>Next Page</b></span>"
Response.Write " | "
Response.Write "<span style=""color:silver;""><b>Last Page</b></span>"
End If
Response.Write "</div>" & vbcrlf
' added these closing tags....
Response.Write "</tr></td>" & vbNewLine
Response.Write "</table>" & vbNewLine
end if
objRS.Close : Set objRS = Nothing
%>
</body>
</html>
|
Dave Maxwell
Barbershop Harmony Freak |
|
|
|
Nikkol
Forum Moderator
USA
6907 Posts |
|
|
davemaxwell
Access 2000 Support Moderator
USA
3020 Posts |
Posted - 05 September 2003 : 12:48:25
|
Well, I usually try not to because everyone's got their own individual style, but this time I saw a bunch of places it had potential errors, so I needed to recode it to be more efficient. Plus getting as much out of the recordset as possible reduces the risk of errors and increases speed significantly with a large set of data.
Hey, I coulda converted it to getrows and concatenated the response.writes and done some other stuff. I did at least TRY and stay within the coding boundries being used.
But if you have some code you want me to look at  |
Dave Maxwell
Barbershop Harmony Freak |
|
|
|
Doug G
Support Moderator
USA
6493 Posts |
Posted - 05 September 2003 : 14:57:31
|
I always use "if rs.bof and rs.eof" to test for an empty recordset. If you make your test immediately after you populate your recordset, using or is reliable since if there is at least one record both eof and bof will be false and the cursor will point at the 1st record. But if you happened to moveprevious on a populated rs before you test for an empty recordset, bof becomes true and if you use "rs.bof or rs.eof" your test will say the rs is empty even if it has records. This would be a rare occurance, and you probably should make sure you don't code things to move around in the rs before you test, but the possibility is there.
From the ms docs:
quote:
If you open a Recordset object containing no records, the BOF and EOF properties are set to True (see the RecordCount property for more information about this state of a Recordset). When you open a Recordset object that contains at least one record, the first record is the current record and the BOF and EOF properties are False.
|
======
Doug G
======
Computer history and help at www.dougscode.com |
|
|
|
Nikkol
Forum Moderator
USA
6907 Posts |
Posted - 05 September 2003 : 17:17:07
|
this is what i do (helps me to never forget to close a recordset):
set rs = conn.execute(strSql)
myArray = Null
if not rs.bof and not rs.eof then myArray = rs.GetRows
rs.Close
set rs = Nothing
if not IsNull(myArray) then
.
.
.
end if |
Nikkol ~ Help Us Help You | ReadMe | 3.4.03 fixes | security fixes ~ |
|
|
|
ruirib
Snitz Forums Admin
Portugal
26364 Posts |
Posted - 05 September 2003 : 20:06:52
|
| Doug, you are right of course. I probably do it because I hardly code something to move backwards right from the first time I start to move around the recordset (in fact I think I never did it). That's why I really haven't needed to worry with this, since moving forward is adequately covered by testing for EOF. |
Snitz 3.4 Readme | Like the support? Support Snitz too |
|
|
|
seahorse
Senior Member
USA
1075 Posts |
Posted - 06 September 2003 : 11:15:31
|
LOL Thanks, Dave. You have no idea how much I appreciate this.
You know, I picked this code up off of the net, becuase I just couldn't get paging to work with mySQL. I started out with Snitz and trying to figure it out, but was was just lost trying to drop the stuff I didn't need.
I was actually ready to bit the bullet and move to MS SQL becuase it was so much easier.
I hope I didn't raise your blood pressure with this |
Ken
===============
Worldwide Partner Group
Microsoft |
|
|
|
davemaxwell
Access 2000 Support Moderator
USA
3020 Posts |
Posted - 06 September 2003 : 11:23:08
|
quote:
Originally posted by seahorse
LOL Thanks, Dave. You have no idea how much I appreciate this.
You know, I picked this code up off of the net, becuase I just couldn't get paging to work with mySQL. I started out with Snitz and trying to figure it out, but was was just lost trying to drop the stuff I didn't need.
I was actually ready to bit the bullet and move to MS SQL becuase it was so much easier.
I hope I didn't raise your blood pressure with this
Nah! I actually enjoy optimizing code for some bizarre reason. I always like trying to find ways to make things better. |
Dave Maxwell
Barbershop Harmony Freak |
|
|
|
seahorse
Senior Member
USA
1075 Posts |
Posted - 06 September 2003 : 11:41:16
|
Well, it works like a charm, Dave. Many thanks to you all.
It's not much, but I thought you and everybody who tried to help deserved some acknowledgement. If not for today, then for help in the past.
http://www.airtoaircombat.com/about.asp
|
Ken
===============
Worldwide Partner Group
Microsoft |
|
|
|
Nikkol
Forum Moderator
USA
6907 Posts |
|
|
Topic |
|
Topic Locked
