http://ayende.comAyende @ RahienCopyright (C) Ayende Rahien 2004 - 2021 (c) 202660I was checking whether someone provided an answer on this one yet... Looking back, please forget my previous incredibly stupid comment; I was talking rubbish;allowing an app to set up a multitude of concurrent connections would hog your DB connection pool, leaving no connections for your app, in other words this would be a recipe for disaster. Rather odd that I missed it the first time, as did everyone else... So, next to the missing "else throw" in the first exception handler, just ditch the paralleled approach all together, as it would probably hog up all connections on your database !http://ayende.com/131073/memorable-code#comment54http://ayende.com/131073/memorable-code#comment54Tue, 22 Nov 2011 12:18:32 GMTI think that for db connections async dev might be beneficial, although I have not tried it. However, I would just opt for a backgroundworker instead. IMO the biggest problem with this code is in the exception handling: it lacks an "else throw;" in case the exception isn't the one expected...http://ayende.com/131073/memorable-code#comment53http://ayende.com/131073/memorable-code#comment53Thu, 10 Nov 2011 20:02:39 GMTDuplicated, Yes, fix now, a post with an explanation coming up shortlyhttp://ayende.com/131073/memorable-code#comment52http://ayende.com/131073/memorable-code#comment52Thu, 10 Nov 2011 08:47:18 GMTIt's your RavenDB database corrupted? Post and comments seems like duplicated (or triplicated... or more) :)http://ayende.com/131073/memorable-code#comment51http://ayende.com/131073/memorable-code#comment51Thu, 10 Nov 2011 07:46:39 GMT@dead-detector He has been teaching classes in NYC. See the top of the sidebar.http://ayende.com/131073/memorable-code#comment50http://ayende.com/131073/memorable-code#comment50Mon, 07 Nov 2011 05:26:53 GMTWhat happend to Oren? Is hope he is alive.http://ayende.com/131073/memorable-code#comment49http://ayende.com/131073/memorable-code#comment49Sun, 06 Nov 2011 10:38:46 GMT@Christopher Thanks for bringing that to my attention. I apologize as I should have been more clear. A slobbering cloud of sycophants. http://ayende.com/131073/memorable-code#comment48http://ayende.com/131073/memorable-code#comment48Sun, 06 Nov 2011 03:40:16 GMT@Clay: you failed to honor Ayende properly in that post. Please enter additional grovel, else you will be reported to the authorities.http://ayende.com/131073/memorable-code#comment47http://ayende.com/131073/memorable-code#comment47Sat, 05 Nov 2011 13:55:16 GMTA cloud of sycophants.http://ayende.com/131073/memorable-code#comment46http://ayende.com/131073/memorable-code#comment46Thu, 03 Nov 2011 19:25:42 GMTA "great" example of cloud computing. A cloud of threads...http://ayende.com/131073/memorable-code#comment45http://ayende.com/131073/memorable-code#comment45Tue, 01 Nov 2011 09:59:11 GMT@Chris Taveres Not necessarily. Any blocking calls should make the thread sleep, at which point the other threads will have an opportunity to run. Which, of course, makes setting ThreadPriority.Highest kind of pointless, but it shouldn't starve out the other worker threads, since most of the time spent here will either be in establishing the connection or executing the command - both of which I would expect to cause the thread to sleep. Unless I'm missing something - I have to admit I'm a bit of a multithreading neophyte.http://ayende.com/131073/memorable-code#comment44http://ayende.com/131073/memorable-code#comment44Tue, 01 Nov 2011 08:16:11 GMT@Harry Agreed. And frequently even when Ayende does provide feedback on the code samples he presents, the comments include lots of additional feedback that he doesn't mention. In the case of code whose shortcomings actually *require* significant expertise to really appreciate, then it makes sense for Ayende to chime in. In the case of a code sample like this that's just egregiously broken, it makes perfect sense to just let the commenters evaluate it. It's like crowdsourcing source code review.http://ayende.com/131073/memorable-code#comment43http://ayende.com/131073/memorable-code#comment43Tue, 01 Nov 2011 07:58:02 GMTAt least an invalid server name in the connection string will timeout in parallel! meh.http://ayende.com/131073/memorable-code#comment42http://ayende.com/131073/memorable-code#comment42Mon, 31 Oct 2011 20:48:34 GMTNot opening the connection is a legitimate bug that you find out about after the first run. Not using a transaction is a huge bug that is not as easy to find. And no way someone wrote this code during a 30 minutes interview and knew the codes of specific sql exceptionshttp://ayende.com/131073/memorable-code#comment41http://ayende.com/131073/memorable-code#comment41Sun, 30 Oct 2011 23:45:17 GMTIf somebody can code this in 30 minutes then either he is really good or really bad; generally it is the later because it include premature optimization code that is obscure.http://ayende.com/131073/memorable-code#comment40http://ayende.com/131073/memorable-code#comment40Sun, 30 Oct 2011 10:01:01 GMTIf the insert throws an exception then it must be an update haha. Love ithttp://ayende.com/131073/memorable-code#comment39http://ayende.com/131073/memorable-code#comment39Sat, 29 Oct 2011 19:56:44 GMTA piece of code and everyone starts laughing. But what is the context of this code? If it is a test for an interview without any VS at hand and just 30 minutes time for it? In this case it isn't that bad. And it would excuse that there is no con.Open...btw most of the commenters above did not realise this. http://ayende.com/131073/memorable-code#comment38http://ayende.com/131073/memorable-code#comment38Sat, 29 Oct 2011 11:54:40 GMTThis code is basically doing nothing but spin a thread for each line within the text file that it is reading. It never open the connection to sql in the first place therefore none of the catches will be executed after it fails on executeNonQuery.http://ayende.com/131073/memorable-code#comment37http://ayende.com/131073/memorable-code#comment37Fri, 28 Oct 2011 19:30:10 GMTI think the biggest problem with this code is that it expresses no intent. I read the code yet I don't have any idea what it does. Yeah, there might be issues with the implementation, but even without ToString() or the threading or whatever this would still be bad code. Good code can be understood.http://ayende.com/131073/memorable-code#comment36http://ayende.com/131073/memorable-code#comment36Fri, 28 Oct 2011 19:09:51 GMTto those who suggested select and then insert/update, it's more efficient to do update, check the return value to see if any row was updated, if it's 0 then do an insert. @Paulo Abom Pinto - in 99% of the times, there's no justification for using a stored procedure to do anything.http://ayende.com/131073/memorable-code#comment35http://ayende.com/131073/memorable-code#comment35Fri, 28 Oct 2011 16:12:57 GMT@EDev of course the SELECT before it's not he perfect way, but he could ensure that on SELECT the record is locked or try a dabase trasaction and try to ensure the flow. In my opinion some of this code should be in a Store Procedure and the thread flow in the C#.http://ayende.com/131073/memorable-code#comment34http://ayende.com/131073/memorable-code#comment34Fri, 28 Oct 2011 15:04:06 GMTA very nice new API - upsert.http://ayende.com/131073/memorable-code#comment33http://ayende.com/131073/memorable-code#comment33Fri, 28 Oct 2011 14:52:11 GMT@Dan Bailiff and @Bob Eventually you'll figure out only the k00l kidz hang out here and they all try to out do each other to impress the guy who runs the blog. You'll need to pass your Ayende blog readers certification test before you'll be accepted as a k00l kid. Fear not, most of them don't understand either - they're just trying to impress each other. Yawn, how boring. It's like living high school over and over. Occasionally there is a nice post, but each time you see code dumps, skip the post, the only responses are from kidz trying to impress each other.http://ayende.com/131073/memorable-code#comment32http://ayende.com/131073/memorable-code#comment32Fri, 28 Oct 2011 14:33:10 GMTNot so memorable ;) For example, if you start the thread in the second loop you can have a really memorable code !!! ;) no limit to the worst!http://ayende.com/131073/memorable-code#comment31http://ayende.com/131073/memorable-code#comment31Fri, 28 Oct 2011 10:09:24 GMTnice :) This is obviously production code for bank accounts and may be the reason for the worlds economic woes.. When all you have is a hammer, everything looks like a nail. http://ayende.com/131073/memorable-code#comment30http://ayende.com/131073/memorable-code#comment30Fri, 28 Oct 2011 07:50:23 GMTHeh... all that effort to get parallelism and he shot it in the foot. Using ThreadPriority.Highest means that the worker thread will take higher priority than any other thread in the process... including the process that's running Main and scheduling the other threads. Once the thread quantum expires, no more iterations on the loop until the scheduled (and probably small number of) worker threads all finish. http://ayende.com/131073/memorable-code#comment29http://ayende.com/131073/memorable-code#comment29Fri, 28 Oct 2011 04:15:56 GMTNo more posts in queue!http://ayende.com/131073/memorable-code#comment28http://ayende.com/131073/memorable-code#comment28Fri, 28 Oct 2011 03:56:31 GMTBob - if he had tried to explain everything that is wrong with this code it would have been a very long post. and may be i'm way off, but i think that even the newest programmer should know the basics of exception handling, magic numbers transactions, threading and the rest. those who don't notice the problems with this code should read some more basic programming articles.http://ayende.com/131073/memorable-code#comment27http://ayende.com/131073/memorable-code#comment27Thu, 27 Oct 2011 23:20:28 GMT@Dan Bailiff & Bob, Keep in mind, this is *Oren's* blog. He can help or not as he sees fit. Personally, just sifting through the comments has helped me pick out several issues I missed on my first read. That lead me to thinking about how I would re-architect to fix those problems in addition to the ones I spotted instantly. That alone is useful.http://ayende.com/131073/memorable-code#comment26http://ayende.com/131073/memorable-code#comment26Thu, 27 Oct 2011 22:23:35 GMTBob: come on, man! He never promised to be didactic in every post! Ayende: thanks - that was awesome. http://ayende.com/131073/memorable-code#comment25http://ayende.com/131073/memorable-code#comment25Thu, 27 Oct 2011 22:09:49 GMT