Derik Whittaker

Syndication

News


Images in this post missing? We recently lost them in a site migration. We're working to restore these as you read this. Should you need an image in an emergency, please contact us at imagehelp@codebetter.com
Fun code of the day

Today I was trying to run some code and I could not figure out why it was failing.  When I looked at the logs I noticed it would run one time, then fail.  Took me a minute to realize why this was failing, but finally it hit me.  (No, this is not my code, in fact today was the first time I have ever seen this is my life :()

Can anyone see what is wrong?

The error message was -- Procedure or function updateSubmitQDocID has too many arguments specified.

SqlCommand insComm = new SqlCommand();
foreach (DataRow row in dt.Rows)
{
 
    ...
    ...
    ...
	
    if (conn != null && conn.State != ConnectionState.Open)
    {
        conn.Open();
    }

    // updates the entry for the doc ID in the SubmitQ to 1
    insComm.CommandText = "updateSubmitQDocID";
    insComm.CommandType = CommandType.StoredProcedure;
    insComm.Parameters.Add("@docID", SqlDbType.VarChar, 30);
    insComm.Parameters["@docID"].Value = row["docID"].ToString();
    insComm.Connection = conn;
    insComm.ExecuteNonQuery();
        
    ...
    ...
    ...
}


Just love find good nuggets of fun code.

Till next time,


Posted 03-04-2008 7:38 AM by Derik Whittaker
Filed under:

[Advertisement]

Comments

Garry Shutler wrote re: Fun code of the day
on 03-04-2008 9:09 AM

It adds the @docId parameter each loop through

Edward wrote re: Fun code of the day
on 03-04-2008 9:11 AM

Tut, tut, tut.

Adding a fresh parameter to the same command object inside a loop.  Might want to set CommandText, CommandType and create the parameter before the loop, then modify the Value only within the loop.

Derik Whittaker wrote re: Fun code of the day
on 03-04-2008 9:32 AM

Yea, sadly i was told that this has been a known issue for some time.  'It works the first time through every time, so we did not fix it' was what i was told.

It has now been fixed.

Love fun little nuggets like this though.

LaptopHeaven wrote re: Fun code of the day
on 03-04-2008 9:47 AM

insComm is never reinitialized.  It used the same object for each iteration of the loop.  It tried to re-add the docId parameter each time.

I would create a BuildCommand method, which generates a new SqlCommand each time.

BuildCommand(string cmdText, CommandType cmdType, SqlConnection conn, params SqlParameters [] paramters )

Edward wrote re: Fun code of the day
on 03-04-2008 9:55 AM

The amount of code like this in the wild must be vast and that really worries me.  It looks to me like the copy/paste fairies have been at work while the original dev didn't really understand what he was doing.  

e.g.  The command "insComm", presumably short for "Insert Command" has its CommandText set to "Update(somethin)".  Insert or update? Make up your mind, or just let the copy/paste fairy decide for you.

Also, why attempt to open the connection each iteration of the loop?  Because the copy/paste fairy says so.

I would imagine the app containing this code is a complete nightmare to support or enhance. You must have drawn the short straw today.

Kalpesh wrote re: Fun code of the day
on 03-04-2008 2:41 PM

I am sure, all of us (me included) have written code several times to do such a thing.

Wouldn't it make sense to use a library (or application block) to do these kind of things. Atleast, it will eliminate errors of this kind & one can focus on errors at higher layer i.e. problem domain

for e.g. We don't worry about tcp/ip, http & errors occurring out of it. The lower layer works well & we need to focus on problem domain. In a similar way, we shouldn't be writing the code again & again to do things which has been done already.

Makes sense?

Derek Nponan wrote re: Fun code of the day
on 03-04-2008 4:42 PM

Should call insComm.Parameters.Clear?

El Guapo wrote re: Fun code of the day
on 03-04-2008 6:56 PM

Wow, people are using SqlCommand? Just us in the 90's and try a SLIGHTLY more sophisticated approach, i.e., data access entlib, or even *gasp* an O/R mapper!!! You never would have had this problem. sigh.

I'm also a little disappointed, but not surprised, at the parade of marginal developers who felt like adding a comment to this post with their "me too" analsysis of an obvious and simple bug.

Ayende Rahien wrote re: Fun code of the day
on 03-11-2008 11:28 AM

Derik,

Problems with this code. Direct use of ADO.Net which leads to a lot of repeating code.

I really dislike seeing this type of code.

No transactions.

No batching.

About The CodeBetter.Com Blog Network
CodeBetter.Com FAQ

Our Mission

Advertisers should contact Brendan

Subscribe
Google Reader or Homepage

del.icio.us CodeBetter.com Latest Items
Add to My Yahoo!
Subscribe with Bloglines
Subscribe in NewsGator Online
Subscribe with myFeedster
Add to My AOL
Furl CodeBetter.com Latest Items
Subscribe in Rojo

Member Projects
DimeCasts.Net - Derik Whittaker

Friends of Devlicio.us
Red-Gate Tools For SQL and .NET

NDepend

SlickEdit
 
SmartInspect .NET Logging
NGEDIT: ViEmu and Codekana
LiteAccounting.Com
DevExpress
Fixx
NHibernate Profiler
Unfuddle
Balsamiq Mockups
Scrumy
JetBrains - ReSharper
Umbraco
NServiceBus
RavenDb
Web Sequence Diagrams
Ducksboard<-- NEW Friend!

 



Site Copyright © 2007 CodeBetter.Com
Content Copyright Individual Bloggers

 

Community Server (Commercial Edition)