Login validator class

Question

I'm trying to learn more about object oriented programming and I have a few questions about the class below.
I'm working in Visual Basic.NET

My questions are

1. Is there a better place / way to show the MessageBox.Show?
   
   
2. I'm being forced to use a single Oracle Connection, in my sample cnn is declared with Friend scope so the entire solution can see it. Is there a better way to handle this Connection Object, perhaps I should pass it all about my app as a parameter?
   
   
3. The Sub SaveJobInfo I put this in a Sub so it wouldn't clutter up the ValidateLoginInfo code, was wondering if there is a better way to check for Nulls other than using the .Size property?
   
   
4. Any other ideas that can improve this code?

Public Class LoginValidator
 Friend Shared Function ValidateLoginInfo(jobNum As String, userID As String, ByRef djnInfo As JobNumInfo) As Boolean
 Try
 Dim cmd As New OracleCommand() With {.Connection = cnn, .CommandType = CommandType.StoredProcedure, .CommandText = "owner.packageName.procedureName"}
 cmd.Parameters.Add(New OracleParameter("param1", OracleDbType.NVarchar2, jobNum, ParameterDirection.Input))
 cmd.Parameters.Add(New OracleParameter("param2", OracleDbType.NVarchar2, userID, ParameterDirection.Input))
 cmd.Parameters.Add(New OracleParameter("param3", OracleDbType.NVarchar2, My.Computer.Name.ToUpper, ParameterDirection.Input))
 cmd.Parameters.Add(New OracleParameter("param3b", OracleDbType.NVarchar2, DJN_LOCK_ID, ParameterDirection.Input))
 cmd.Parameters.Add(New OracleParameter("param4", OracleDbType.Varchar2, 20, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param5", OracleDbType.Varchar2, 20, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param6", OracleDbType.Varchar2, 20, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param7", OracleDbType.Varchar2, 10000, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param8", OracleDbType.Varchar2, 10, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param9", OracleDbType.Varchar2, 20, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param10", OracleDbType.Varchar2, 10, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param11", OracleDbType.Varchar2, 2, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param12", OracleDbType.Varchar2, 50, String.Empty, ParameterDirection.Output))
 cmd.Parameters.Add(New OracleParameter("param13", OracleDbType.Varchar2, 1500, String.Empty, ParameterDirection.Output))
 cmd.ExecuteNonQuery()
 SaveJobInfo(djnInfo, cmd)
 For Each p As OracleParameter In cmd.Parameters
 p.Dispose()
 Next
 Catch ex As Exception
 MessageBox.Show("Error occurred in LoginValidator.ValidateLoginInfo" & Environment.NewLine & Environment.NewLine & ex.Message, msgCaption)
 Return False
 End Try
 Return True
 End Function
 Private Shared Sub SaveJobInfo(ByVal djnInfo As JobNumInfo, ByVal cmd As OracleCommand)
 If cmd.Parameters(4).Size > 0 Then
 djnInfo.UserAccessLevel = cmd.Parameters(4).Value.ToString
 Else
 djnInfo.UserAccessLevel = String.Empty
 End If
 If cmd.Parameters(5).Size > 0 Then
 djnInfo.HasChildren = cmd.Parameters(5).Value.ToString.ToUpper = "YES"
 Else
 djnInfo.HasChildren = False
 End If
 If cmd.Parameters(6).Size > 0 Then
 djnInfo.IsReleased = cmd.Parameters(6).Value.ToString.ToUpper = "YES"
 Else
 djnInfo.IsReleased = False
 End If
 If cmd.Parameters(7).Size > 0 Then
 djnInfo.SpecInfo = cmd.Parameters(7).Value.ToString
 Else
 djnInfo.SpecInfo = String.Empty
 End If
 If cmd.Parameters(8).Size > 0 Then
 djnInfo.OrgCode = cmd.Parameters(8).Value.ToString
 Else
 djnInfo.OrgCode = String.Empty
 End If
 If cmd.Parameters(9).Size > 0 Then
 djnInfo.UOMFromOra = cmd.Parameters(9).Value.ToString
 Else
 djnInfo.UOMFromOra = String.Empty
 End If
 If cmd.Parameters(10).Size > 0 Then
 djnInfo.DrawingNumber = cmd.Parameters(10).Value.ToString
 Else
 djnInfo.DrawingNumber = String.Empty
 End If
 If cmd.Parameters(11).Size > 0 Then
 djnInfo.ProductRevisionCode = cmd.Parameters(11).Value.ToString
 Else
 djnInfo.ProductRevisionCode = String.Empty
 End If
 If cmd.Parameters(12).Size > 0 Then
 djnInfo.FirstArticleFamCode = cmd.Parameters(12).Value.ToString
 Else
 djnInfo.FirstArticleFamCode = String.Empty
 End If
 ' param 13 will be handled differently than the others
 End Sub
End Class

• Ethan Bierlein: I've removed the fluff you added to the end of your answer. If you have something to ask, ask it in the comments of the answer.
• Rose: I had about 4 more 'fluffs' i was going to add to this original question so I could could use the formatting. Would it make more sense to make new questions for my other 'fluffs'?
• Ethan Bierlein: As long as your new question contains working, non-hypothetical code that you want reviewed, then ask away! :-)
• Rose: It would be nice if I could keep it all in one question. I'm not sure how to add to this question and have the option to 'format' my fluff additions?
• Ethan Bierlein: Please just ask a new question. It's easier for everyone on the site, and it doesn't invalidate answers.

Answer 1

• My first impression is that this class is doing too much. It is calling the database and manipulating the user interface. This needs to be separated.

• Exceptions are being swallowed and shown to the user, and a boolean value is returned.

  If an exception gets thrown in code calling to the database, let the exception bubble up. This is a catastrophic problem. If validating the login info is throwing an exception, write the database code so that it returns a flag if someone's login info isn't correct. Don't use exceptions for flow control.

• The ValidateLoginInfo method calls SaveJobInfo. After validating login info, you shouldn't be saving data to the database. That should be a separate operation.

• I'm also questioning why you need to explicitly dispose of the OracleParameter objects. The CLR should clean those up just fine on its own.

• The SaveJobInfo method is confusing. It's taking stored procedure parameter values and mapping them to a JobInfo object? This should be done after validating the login info. The stored procedure you are calling has a lot of output parameters. The validation of login info and loading data into the JobInfo object should be two separate calls. One does not have anything to do with the other.

@Rose commented:

If I changed the Exception handling and Caught any exceptions and simply Threw them back to whatever was calling ValidateLoginInfo, would this meet the 'bubbling up' definition?

If you are doing this:

Try
 // ...
Catch ex As Exception
 throw ex
End Try

Then there is no point in using a Try-Catch. Just execute the code and let the exceptions fly.

By "let the exception bubble up" I mean "don't use a Try-Catch if you can't recover from the exception." Unless your ValidateLoginInfo catches an exception and can recover from it, don't catch exceptions.

• Rose: Thank you for the feedback I really appreciate it. In regards to an exception being thrown in the code calling to the database and letting the exception 'bubble up'. . . Please see edit to original question. If I changed the Exception handling and Caught any exceptions and simply Threw them back to whatever was calling ValidateLoginInfo, would this meet the 'bubbling up' definition?