Sunday, October 15, 2006

Why using code samples from the internet is a bad idea

If you're anything like me, you don't know how to make a computer do almost anything, but you do know where to look for code samples that show you how.
Also, if you're anything like me, you don't often have a lot of time to play the code samples, see how they work, read the API documentation (if there is any!), or often even the article text that accompanies the code. Usually it's: google, download code, unzip, compile, run. Then a quick once over to see if it does what I want.
However, sample code should never make it into production. Why? Because it's SAMPLE CODE.
The aims of sample code are to illustrate how a technology/technique works.
What it doesn't aim to do is be ideal code that you can copy and paste straight into your application.
Here's a sample from devx.com:

string connectionString = "Data Source=MEDIACENTER;" +

"Initial Catalog=AdventureWorks;Integrated Security=SSPI;" +

"MultipleActiveResultSets=True";
string strSQLGetOrder = "Select * from Sales.SalesOrderDetail" +
"WHERE SalesOrderID = 43659";


string strSQLUpdateInv = "UPDATE
Production.ProductInventory " +
"SET Quantity=Quantity-@amt WHERE (ProductID=@pid)";

SqlConnection marsConnection = new SqlConnection(connectionString);

marsConnection.Open();

SqlCommand readCommand =
new SqlCommand(strSQLGetOrder, marsConnection);
SqlCommand writeCommand =
new SqlCommand(strSQLUpdateInv, marsConnection);

writeCommand.Parameters.Add
("@amt", SqlDbType.Int);
writeCommand.Parameters.Add
("@pid", SqlDbType.Int);
using (SqlDataReader rdr = readCommand.ExecuteReader())
{
while (rdr.Read())
{
writeCommand.Parameters
["@amt"].Value = rdr["OrderQty"];

writeCommand.Parameters["@pid"].Value = rdr["ProductID"];
writeCommand.ExecuteNonQuery();
}
}
marsConnection.Close ();
If I saw code like this in someone's production code, I'd see red. This is, frankly, shit.
What does is illustrate how you can use Multiple Active Recordsets in ADO.NET 2.0.
What's wrong with it?
  • Embedded SQL.
  • It doesn't clean up the open connection reliably.
  • Embedded column and parameter names.
  • Hard coded connection string.
  • No mention of Transactions.
  • Result would be better achieved in a simple SQL statement.
I could go on. I suppose the main problem with this sample is that it isn't integrated with the rest of your code. Because it is provided as a stand alone sample, it doesn't use any underlying framework that you're using. For example, if I was using the Data Access Application Block to underpin my data access, this code snippet is useless. Also, if I'm using an "n-tier", or layered, design, this code is pretty pointless.
So, we should use code samples to see how to use a specific technology or technique, but then go away and write our own code that fits in with your application.

No comments: