add data to access - fail

D

dek

please review my asp code:

<%
Dim adoCon ''hold database connection object
Dim rsAddRecord ''hold recordset for new record to be added
Dim strSQL ''hold the SQL query to query the database
Dim strConnection ''hold data temp


set adoCon = Server.CreateObject("ADODB.Connection")

Set rsAddRecord=Server.CreateObject("ADODB.Recordset")

adoCon.Open "FILEDSN=D:\dsn\database_dsn.dsn"

strSQL = "SELECT Nama.ID, Nama.Nama, Nama.Umur, Nama.NoTelefon FROM Nama;"
rsAddRecord.LockType=3
rsAddRecord.Open strSQL, adoCon
rsAddRecord.AddNew
rsAddRecord.Fields("ID")=Request.Form("ID")
rsAddRecord.Fields("Nama")=Request.Form("Nama")

rsAddRecord.Update

rsAddRecord.Close
Set rsAddRecord=Nothing
Set adoCon=Nothing
''redirect to the TestData.asp page
Response.Redirect "TestData.asp"
%>
 
R

Ray Costanzo [MVP]

I'd prefer something like this, but with data validation.

Using DSNs is ill-advised.
Creating a recordset object to do an insert is ill-advised.
Your ID column isn't an identity column or some sort of other
auto-incrementing column?

<%
Dim adoCon, strSQL, strConnection
strConnection = "get your connection string at www.connectionstrings.com "

strSQL = "INSERT INTO Nama (ID,Nama) VALUES (" & Request.Form("ID") & ",'" &
Replace(Request.Form("Name"), "'", "''") & "'"
Set adoCon = CreateObject("ADODB.Connection")
adoCon.Open strConnection
adoCon.Execute strSQL,,129
adoCon.Close
Set adoCon = Nothing
Response.Redirect "testdata.asp"
%>


Ray at home

Set adoCon = CreateObject("ADODB.Connection")
 
B

Bob Barrows [MVP]

dek said:
please review my asp code:

<%
Dim adoCon ''hold database connection object
Dim rsAddRecord ''hold recordset for new record to be added
Dim strSQL ''hold the SQL query to query the database
Dim strConnection ''hold data temp


set adoCon = Server.CreateObject("ADODB.Connection")

Set rsAddRecord=Server.CreateObject("ADODB.Recordset")

adoCon.Open "FILEDSN=D:\dsn\database_dsn.dsn"

strSQL = "SELECT Nama.ID, Nama.Nama, Nama.Umur, Nama.NoTelefon FROM
Nama;"

This is bad: you are retrieving all te data in the table without intending
to use any of it.
rsAddRecord.LockType=3
rsAddRecord.Open strSQL, adoCon
rsAddRecord.AddNew

This is a very inefficient way to add a record to your table.
rsAddRecord.Fields("ID")=Request.Form("ID")
rsAddRecord.Fields("Nama")=Request.Form("Nama")

Why did you retrieve Umur and NoTelefon?
rsAddRecord.Update

rsAddRecord.Close
Set rsAddRecord=Nothing
Set adoCon=Nothing
''redirect to the TestData.asp page
Response.Redirect "TestData.asp"
%>

I would prefer this:

Dim cn 'no need for long variable name - "cn" is universal
Dim cmd 'Command object variable
Dim arParms 'array to hold parameter values
Dim strSQL
Dim strConnection

strConnection = "<ole db connection string>"
'see www.able-consulting.com/ado_conn.htm
'or www.connectionstrings.com

strSQL = "INSERT INTO Nama (ID,Nama) VALUES (?,?)"
arParms =Array(Request.Form("ID"), Request.Form("Nama"))

set cn=createobject("adodb.connection")
cn.open strConnection

set cmd=createobject("adodb.command")
cmd.CommandText=strSQL
set cmd.ActiveConnection=cn
cmd.Execute ,arParms, 129
set cmd=nothing
cn.close:set cn=nothing

I prefer using a Command object to pass parameters to your sql statement vs.
using dynamic sql (concatenation) because
1. It prevents hackers from using sql injection to hack your database
2. It's easier to write the code for this since you don't have to worry
about delimiters (quotes)
3. It performs the slightest bit faster than dynamic sql

Bob Barrows
PS. The 129 in the cmd.Execute statement is the result of the addition of 2
constants:

1 - adCmdText - Tells ADO that you are executing a sql string - you should
use this setting when opening a recordset as well:
Set rs = cn.Execute(strSQL,,1)

128 - adExecuteNoRecords - Tells ADO that it does not have to construct a
recordset object since the query being executed does not return records. If
you do not specify this setting, ADO will waste time and resources creating
a recordset object that will never be used
 

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
474,261
Messages
2,571,041
Members
48,769
Latest member
Clifft

Latest Threads

Top