Can I just say, YUCK. Check this out, here are the awful downsides of the supplied "sample" & "ideal" code, straight from "otn.oracle.com" (ooh !!).
1. So there is no layered design. The OracleConnection sits as a private member variable of the webservice. Fine this is a sample code thingie - thats allright, frictionless surface example, I understand that, this is pardonable, but wait it gets better.
2. Of course the webservice (ASMX) that runs on a webserver, that holds the OracleConnection as a "protected shared" global to the class variable - it's connection opening and closing - is controlled by the remote client using two WebMethods - OpenConnection/CloseConnection. (And the code of these two makes me puke)
3. Here is a minor issue which I personally could give a rats head about "Variable Naming", as long as it's not horrific, but who names their methods in Camel case? And half their variables in hungarian and the rest of the half in god knows what. But if the core code were functionally okay, and understandable enough - I'd say - who cares. But WTF is "ocb"? Oh OracleCommandBuilder - wait this is getting bettter worse as I type.
4. So there are two WebMethods "OpenConnection" and "CloseConnection" right? Oh wait it gets bettter worse. The CloseConnection is called from the client side ASPX. You think that is bad? Oh you haven't seen the real crappy part yet - The CloseConnection WebMethod is called using the OnClick of a button on an ASPX. So not only is the connection open for - well - if you went for lunch for instance, but the connection remains open if I close the browser and go home, and then your session times out (20 minutes), and then two cycles of garbage collector run (approx 8-10 minutes), and finally the OracleConnection is finalized. YUCK.
5. So how does CloseConnection work? Check this code snippet out -
<WebMethod()> Public Function closeConnection() As Boolean
Try
con.Dispose()
con.Close()
Catch ex As Exception
Finally
'Set connected to false
connected = False
End Try
closeConnection = connected
End Function
So Dispose is before close, and if an exception occurs - just ignore it? eat up the exception/do nothing with it?
6. And whats up with that "connected = False" . Oh wait, that is a global boolean - oh this is getting even worse. Why not use .. umm .. the CONNECTIONSTATE !!
7. And, oh look there is a WebMethod that runs DDL - Yes sir, Create Table/Drop Table, right over a web service.
8. And why are all the global variables "Protected Shared" ???? Oh wait, I know why - because the code sucks !!!
9. And whatsup with "Creating an ocb - Oh wait that is OracleConnectionBuilder" - and then not using it - TWICE? Oh god !!!
10. Oh look, no parameterized SQL, concatenation of strings --- Ooooh !! I love the smell of Injection attack this time of night.
11. And I love their connection string -
conStr = New String("User Id=" + username + ";Password=" + password + ";Data Source=" + datasource + ";")
can u say - "STRINGBUILDER" ??
12. Oh wait dude, look at the full code -
<WebMethod()> Public Function OpenConnection(ByVal username As String, ByVal password As String, ByVal datasource As String) As Boolean
Try
'Create OracleConnection instance
con = New OracleConnection()
'Create the connect string
conStr = New String("User Id=" + username + ";Password=" + password + ";Data Source=" + datasource + ";")
'Set the connect String
con.ConnectionString = conStr
'Try to open the connection
con.Open()
'Set connected to true
connected = True
Catch ex As Exception
'An error ocurred. Set connected to false
connected = False
End Try
'Return the connection status True or False
OpenConnection = connected
End Function
So let me get this straight, the remote user is passing in a userName, passWord, and dataSource over a webservice? OH wow, this code is smelling worse than an anchovy's smelly bundt (cake). Seriously, this code is making me puke. And the exception is being eaten, and again that "connected" boolean.
Okay, in about a 2 page code sample, in 5 minutes, I could spot 12 serious shortcomings. And I yet haven't even looked at the client code in depth. Now here is the icing on the cake.
Check the title of the page I got this sample from -- "Advanced ODP.NET Samples". I am sorry Oracle, but if this is the quality of your "advanced code samples" on OTN, I am scared to ask what might be inside ODP.NET.
Oh and don't think I found this page at an obscure corner on Oracle.Com. Try this -
1. Go to http://otn.oracle.com
2. On the top menu click on Sample Code -> ODP.NET
3. Click on "Advanced ODP.NET Samples", what I described is Example #2, the rest aren't much better either.
This code has seriously damaged my fragile mind. Sahil must get some sleep. Oh my, this has to be a dream!!