mmm I'm sure that I have because an identical set of code and functions for another part of the app works fine and this is the way I have been taught to insert data into the SQL database whilst keeping the front end blissfully unaware of the logic required to enter the data into the database.
You cant really say "oh, i copied and pasted this from another part of the program where it works perfectly so it must work perfectly in this context too" - copy/paste assumptions are one of the biggest introducers of subtle bugs...
The reason why I'm using an XML schema to build a DATASET is that if changes need to be made as to what data is to be inserted into the database (and believe me there are regular changes) then using a Schema means that I just adjust the schema to include the new items and then slightly alter the front end code to include the new items.
by using the datatable method above I would have to alter a lot more of the code to include any changes.. or at least thats my perspective on it.
Its something of a mystery to me, after going to the effort of creating and using a typed dataset, that you would go and use it in generic fashion..
Try
objAddressData.PupilAddressDetails
Instead of:
objAddressData.Tables("PupilAddressDetails")
Things get a lot clearer because the intellisense is able to work out what youre talking about! Try it and see - youll see column definitions, names, types etc- it can really help prevent runtime errors of stuffing a string into a boolean..
Also using a dataset I find it better for performance as it does not need to have a constant connection to the database open. (I think lol)
Not strictly true - there are a constant number of lingering connections open to the database that are re-used as and when the .NET framework feels like it. How it manages them is not our concern, but suffice to say its like a pool of hire cars.. when you Open() a connection (after the first time) a connection is leased from the pool, rather than constructed from nothing, used and when Close()d or Dispose()d it is returned to the pool rather than being destroyed (it would be very inefficient to scrap and rebuild a car after every journey)
any suggestions about other methods on using a DATASET?
Few suggestions on dataset as well as other aspects:
dont prefix your variable names with "obj" - in an OO programming language like .NET, everything is an object (apart from primitives like int, boolean) so youre not really adding any value with that prefix. You would be better suffixing the variable with something that indicates its type:
addressDataDS for example
-
As noted before - youve gone to the effort of creating a typed dataset, dont access it in untyped fashion:
untyped= DirectCast(myDataSet.Tables("MyTableName").Columns("MyBooleanColumn"), DataColumn).DefaultValue = "wrongly try to put a string in a boolean"
typed= myDataSet.MyTableName.MyBooleanColumn.DefaultValue = "oops, thats not a boolean!"
Only in the second case will the IDE warn you before your program crashes at runtime.
Same for the NewRow stuff:
Dim ro as MyTypedDataRow
<-- look, it's a TYPED data row.. this is cool because:
ro = MyTypedDataSet.MyTypedDataTable.New
MyTypedDataRowRow()
ro.MyBooleanColumn = true
ro.MyIntColumn = 1
ro.MyStringColumn = "hello world"
definitely better than this:
Dim ro as DataRow
ro = MyTypedDataSet.Tables("MyTypedDataTableName").NewRow()
ro.Items("MyBooleanColumm") = false
ro.Items("MyIntColem") = 1
ro.Items("MyStirngCollum") = "oops i made a typo in all the column names"
youll have to wait till runtime to find those bugs...
-
When you want to find out soemthing that is null, use the debugger!
so your line:
Dim objDataRow As DataRow = objAddressData.Tables("PupilAddressDetails").NewRow
it crashes.. no problems. Open the immediate window and type:
?objdatarow
Nothing <-- its nothing.. ok, it hasnt been assigned - the call to function didnt complete
?objAddressData
Nothing <-- ohhh, our dataset is nothing! no wonder it crashed when we try to call a method on it!
So go single step into the function that is supposed to return a nice working data set. Go examine it...
-
More suggestions:
Public Function AddPupilAddress(ByVal PupilAddressDetails As DataSet) As Boolean
Try
'call the data component to add the new address
Return objMIWSDAPupils.AddPupilAddress(PupilAddressDetails)
[B]Catch ExceptionErr As Exception[/B]
[B] Throw New System.Exception(ExceptionErr.Message, ExceptionErr.InnerException)[/B]
End Try
End Function
Ermm.. Why do you catch an exception only to go and throw it again? You havent changed the type of exception here or done any processing of it.. dont bother with this try/catch at all if youre only going to create another identical exception from this exception and throw that.
Especially do not do this:
[I] Catch ExceptionErr As Exception[/I]
[I] Throw New System.Exception[/I]
[I] End Try[/I]
You catch one exception with a potentially meaningful error message, and re-throw a blank, no-message exception. The simple rule is, if youre going to use the word "Throw" in a "Catch" block, consider whether that catch block should be there at all..
-
Public Function GetNewAddressDS() As DataSet
Try
'return the mainpupilsdetails schema as a dateset
[B]GetNewAddressDS = New PupilAddressDetails[/B]
Decide on a consistent way of doing your returns.. In the last example it was:
Return (value)
In this one it's
FunctionName = (value) [and then wait for function to end]
I recommend you settle on "return" as a way of exiting subs, functions and "return (value)" to return values (from functions) because:
a) its the way most other languages operate
b) its cleaner
-
Private Const ImageSelected As Integer = 0
Private Const ImageUnselected As Integer = 1
Private Const ImageHighlighted As Integer = 2
Private Const ImagePupils As Integer = 7
Private Const ImagePreAdmissionPupils As Integer = 8
Private Const ImageStaff As Integer = 9
Private Const ImageContacts As Integer = 10
Private Const ImageDoctors As Integer = 11
Private Const ImagePreAdmissionGroups As Integer = 12
This is what we now call an Enum:
Private Enum ImageIndexes As Integer
ImageSelected = 0
ImageUnselected = 1
etc...
End Enum
Enums are cool because they are type safe and they m,ean something. You can store the string in a database and get VB to parse it back into an enum value for you. They give meaning to your code instead of all these random magic numbers flying around
If you create a method that requires an enum:
Public Sub SetImage(idx as ImageIndexes)
now we cant just say:
setImage(4) <--magic number!
before.. there was nothing stopping us doing that, with just raw constant ints. Look Enums up some time.. handy things..
-
re "data access logic function"
--> the idea of parameterized queries is partly to protect against sql injection hacking (which yours does) and partly that you buiild them once, set all the parameter types once, and then reuse the command jsut by changing the values and running it again (which you dont do)
Your code, builds the command, parameters etc, loads of new objects.. runs the query then throws it all away again.. You mentioned efficiency as part of your notion of typed datasets, so surely a constant logic of creating an destroying large numbers of objects that are optimized for preservation, will be of interest
-
Do not use the "Call" keyword - it is no longer needed
-
Last but not least.. i know it looks like i hammered on just about every aspect of the posted code.. hope you didnt get offended by it!
