Is this a bad Idea?

  • Thread starter Thread starter Michael Konopka
  • Start date Start date
M

Michael Konopka

Can someone tell me what might go wrong with the following code. It
seems to work fine for me but I'm doubtfull.

I'm adding data to a MSAccess database with an autonumber column
called "id".
This is a single user app, so there will never be more than one
connection to the database.

I've seen posts where people queried "@@identity" within an RowUpdated
event handler for the DataAdapter but I don't want that..so I just put
the query for "@@identity" in line after the call to Update().

The code i'm curious about is just before the catch.

public int CreateNewOrder(int operatorID,short typeID)
{
OleDbCommand dbc = null;
OleDbDataAdapter da = null;
OleDbCommandBuilder cb = null;
DataSet dataSet = null;
DataRow row = null;

try
{

da = new OleDbDataAdapter("select * from [order]",m_dbConn);
da.SelectCommand.Transaction = m_dbTrans;
cb = new OleDbCommandBuilder(da);
cb.QuotePrefix = "[";
cb.QuoteSuffix = "]";

dataSet = new DataSet();
da.Fill(dataSet);

row = dataSet.Tables[0].NewRow();
row["operator_id"] = operatorID;
row["type_id"] = typeID;
row["status_id"] = (short)Util.Constants.OrderStatus.OPEN;
row["date_created"] = DateTime.Now;
row["date_modified"] = DateTime.Now;
dataSet.Tables[0].Rows.Add(row);

DataRow[] dra = dataSet.Tables[0].Select(null, null,
DataViewRowState.Added);
da.Update(dra);


da.Dispose();

dbc = new OleDbCommand("SELECT @@IDENTITY",m_dbConn);
dbc.Transaction = m_dbTrans;
return (int)dbc.ExecuteScalar();

}
catch(Exception e)
{
m_bErrored = true;
throw e;
}
finally
{
if(dbc != null)
try{dbc.Dispose();}
catch(Exception){}

if(dataSet != null)
try{dataSet.Dispose();}
catch(Exception){}

if(cb != null)
try{cb.Dispose();}
catch(Exception){}

if(da != null)
try{da.Dispose();}
catch(Exception){}


}
}
 
Hi,

It might work if you have only one insert at time.
Btw, why are you putting Dispose calls into try/catch block?
You might also use "using" statement as a neat way of disposing.
 
The try catch blocks around the dispose calls are because I don't know
better and I'm very very new with C# / .Net and well.. I figured it
couldn't hurt.
I'm not sure when I'm supposed to call Dispose() anyway (all the time
when I'm done with an object or ??). I haven't found anything to help
clarify my Dispose() dilemma.

What do you mean about the "using" statement? I thought that was the
same as Java's "import" statement. How can the using statement help
dispose objects?




Miha Markic said:
Hi,

It might work if you have only one insert at time.
Btw, why are you putting Dispose calls into try/catch block?
You might also use "using" statement as a neat way of disposing.

--
Miha Markic [MVP C#] - RightHand .NET consulting & development
miha at rthand com
www.rthand.com

Michael Konopka said:
Can someone tell me what might go wrong with the following code. It
seems to work fine for me but I'm doubtfull.

I'm adding data to a MSAccess database with an autonumber column
called "id".
This is a single user app, so there will never be more than one
connection to the database.

I've seen posts where people queried "@@identity" within an RowUpdated
event handler for the DataAdapter but I don't want that..so I just put
the query for "@@identity" in line after the call to Update().

The code i'm curious about is just before the catch.

public int CreateNewOrder(int operatorID,short typeID)
{
OleDbCommand dbc = null;
OleDbDataAdapter da = null;
OleDbCommandBuilder cb = null;
DataSet dataSet = null;
DataRow row = null;

try
{

da = new OleDbDataAdapter("select * from [order]",m_dbConn);
da.SelectCommand.Transaction = m_dbTrans;
cb = new OleDbCommandBuilder(da);
cb.QuotePrefix = "[";
cb.QuoteSuffix = "]";

dataSet = new DataSet();
da.Fill(dataSet);

row = dataSet.Tables[0].NewRow();
row["operator_id"] = operatorID;
row["type_id"] = typeID;
row["status_id"] = (short)Util.Constants.OrderStatus.OPEN;
row["date_created"] = DateTime.Now;
row["date_modified"] = DateTime.Now;
dataSet.Tables[0].Rows.Add(row);

DataRow[] dra = dataSet.Tables[0].Select(null, null,
DataViewRowState.Added);
da.Update(dra);


da.Dispose();

dbc = new OleDbCommand("SELECT @@IDENTITY",m_dbConn);
dbc.Transaction = m_dbTrans;
return (int)dbc.ExecuteScalar();

}
catch(Exception e)
{
m_bErrored = true;
throw e;
}
finally
{
if(dbc != null)
try{dbc.Dispose();}
catch(Exception){}

if(dataSet != null)
try{dataSet.Dispose();}
catch(Exception){}

if(cb != null)
try{cb.Dispose();}
catch(Exception){}

if(da != null)
try{da.Dispose();}
catch(Exception){}


}
}
Click to expand...
Click to expand...
 
Michael Konopka said:
The try catch blocks around the dispose calls are because I don't know
better and I'm very very new with C# / .Net and well.. I figured it
couldn't hurt.
I'm not sure when I'm supposed to call Dispose() anyway (all the time
when I'm done with an object or ??). I haven't found anything to help
clarify my Dispose() dilemma.
Click to expand...

The best practice is to dispose the object ASAP.
What do you mean about the "using" statement? I thought that was the
same as Java's "import" statement. How can the using statement help
dispose objects?
Click to expand...

There is another use for using keyword:

using (OleDbDataAdapter da = new OleDbDataAdapter("select * from
[order]",m_dbConn))
{
.. do something with da
}

The code above guarantees that da gets disposed at the end of the block.

--
Miha Markic [MVP C#] - RightHand .NET consulting & development
miha at rthand com
www.rthand.com

"Miha Markic [MVP C#]" <miha at rthand com> wrote in message
Hi,

It might work if you have only one insert at time.
Btw, why are you putting Dispose calls into try/catch block?
You might also use "using" statement as a neat way of disposing.

--
Miha Markic [MVP C#] - RightHand .NET consulting & development
miha at rthand com
www.rthand.com

Michael Konopka said:
Can someone tell me what might go wrong with the following code. It
seems to work fine for me but I'm doubtfull.

I'm adding data to a MSAccess database with an autonumber column
called "id".
This is a single user app, so there will never be more than one
connection to the database.

I've seen posts where people queried "@@identity" within an RowUpdated
event handler for the DataAdapter but I don't want that..so I just put
the query for "@@identity" in line after the call to Update().

The code i'm curious about is just before the catch.

public int CreateNewOrder(int operatorID,short typeID)
{
OleDbCommand dbc = null;
OleDbDataAdapter da = null;
OleDbCommandBuilder cb = null;
DataSet dataSet = null;
DataRow row = null;

try
{

da = new OleDbDataAdapter("select * from [order]",m_dbConn);
da.SelectCommand.Transaction = m_dbTrans;
cb = new OleDbCommandBuilder(da);
cb.QuotePrefix = "[";
cb.QuoteSuffix = "]";

dataSet = new DataSet();
da.Fill(dataSet);

row = dataSet.Tables[0].NewRow();
row["operator_id"] = operatorID;
row["type_id"] = typeID;
row["status_id"] = (short)Util.Constants.OrderStatus.OPEN;
row["date_created"] = DateTime.Now;
row["date_modified"] = DateTime.Now;
dataSet.Tables[0].Rows.Add(row);

DataRow[] dra = dataSet.Tables[0].Select(null, null,
DataViewRowState.Added);
da.Update(dra);


da.Dispose();

dbc = new OleDbCommand("SELECT @@IDENTITY",m_dbConn);
dbc.Transaction = m_dbTrans;
return (int)dbc.ExecuteScalar();

}
catch(Exception e)
{
m_bErrored = true;
throw e;
}
finally
{
if(dbc != null)
try{dbc.Dispose();}
catch(Exception){}

if(dataSet != null)
try{dataSet.Dispose();}
catch(Exception){}

if(cb != null)
try{cb.Dispose();}
catch(Exception){}

if(da != null)
try{da.Dispose();}
catch(Exception){}


}
}
Click to expand...
Click to expand...
Click to expand...
 
Back
Top