HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlMinor

Update 12 records in a database table in less than 13 queries

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
updaterecordsthandatabaselessqueriestable

Problem

I've got a database table in my application called "periods". The structure of the table is:

  • Year (Number)



  • Month (Number)



  • Period (Text)



The Year column stores years (Say, 2016), Month stores months (1-12), and period stores the period of the month/year - eg; November 2016 = 11/16, so period = 1116.

In the table, it stores from 2016, to 2030, so 14 years worth.

In my code I currently have the following Try statement on my main forms Load event. The aim of this code is that on the 1st January each year, the table updates and sets the Year column to 15 years in advance where year = current year -1, and then creates the periods.

It currently uses one update for each record - Is there a way I can reduce the code here to less queries, but still get the desired effect?

For instance, 01/01/2017 will set all Year values to 2031 WHERE Year = 2016, and then (this is the part I struggled with), set the periods to correspond with the month and year of that record?

`Try
Dim current As String = Convert.ToString(Date.Today)
Dim year As String = Convert.ToString(Date.Today.AddYears(-1).Year)
Dim nextyear As String = Convert.ToString(Date.Today.AddYears(15).Year)
Dim period As String
current = current.Substring(0, 5)
period = nextyear.Substring(2, 2)

If current = "01/01" Then
trans = con.BeginTransaction()
sql = "UPDATE [periods] SET [year] = ? WHERE [year] = ?"

Dim cmd As New OleDbCommand(sql, con, trans)
cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
cmd.Parameters.Add("@y", OleDbType.Integer).Value = year
cmd.ExecuteNonQuery()

sql = "UPDATE [periods] SET [period] = ? WHERE [month] = ? AND [year] = ?"

cmd = New OleDbCommand(sql, con, trans)
cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "01" & period
cmd.Parameters.Add("@month", OleDbType.Integer).Value = 1
cmd.Parameters.Add("@year", OleDbType.Integer).V

Solution

Dim current As String = Convert.ToString(Date.Today)
Dim year As String = Convert.ToString(Date.Today.AddYears(-1).Year)
Dim nextyear As String = Convert.ToString(Date.Today.AddYears(15).Year)
Dim period As String
current = current.Substring(0, 5)
period = nextyear.Substring(2, 2)


It's not clear why you would want to work with String variables, given...

cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear


The database wants an Integer type, so the Convert.ToString conversions are useless at best, since you're going to be boxing the value type into an Object anyway.

Dim current As Date = Date.Today
Dim year As Integer = Date.Today.AddYears(-1).Year
Dim nextyear As Integer = Date.Today.AddYears(15).Year


It's not clear what's going on with the next two instructions:

current = current.Substring(0, 5)
period = nextyear.Substring(2, 2)


First, that's re-assigning current, which has been assigned a value that was never used. Next, these assignments are extracting specific parts of a Date represented as a String, but you have no control on that representation - your code might work in an en-US locale, but produce nonsense results in a fr-FR or en-CA locale.

I'm not sure what the value of current should be after that second assignment. Assuming say 2016/11/02 (or would it be 11/02/2016?) so the 5 left-most characters would be either 2016/ or 11/02 (or 02/11?), and we have no way of telling by just reading the code.

The period would be 11 I guess (I'm starting to presume your date format looks like 02/11/2016), but then again I can't be 100% sure just by reading the code.

It's not clear why next year would be 15 years from now either: I find year, nextyear and current could use better, more descriptive and less misleading names.

Now, without changing the actual SQL queries, you code could use a loop, to parameterize the @period value:

For month = 1 To 12
    cmd = New OleDbCommand(sql, con, trans)
    cmd.Parameters.Add("@period", OleDbType.VarChar).Value = String.Format("{0:D2}{1}", month, period)
    cmd.Parameters.Add("@month", OleDbType.Integer).Value = month
    cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
    cmd.ExecuteNonQuery()
Next


Now, this will clean up your code a lot, but will still send 12 queries to the database server.

Looks like you can derive the @period parameter from the @month value, right?

sql = "UPDATE [periods] SET [period] = [month] WHERE [year] = ?"


Of course that won't work as-is. But the point is that you'll want to tweak the period assignment to derive its value from the month field on the same record, using data that's already in the database. That way you'll update all records for the specified year in a single query.

You haven't shown where the con object is initialized or closed, but it should be in a Using block, so that it's properly closed and released when you're done with it. Same with the OleDbCommand instances: the class implements the IDisposable interface, so you should either explicitly call .Dispose, or wrap its usage in a Using scope. Doing that would also make your code more sane - by reusing the cmd object you're giving it a new meaning everytime, which is never a good idea.

Code Snippets

Dim current As String = Convert.ToString(Date.Today)
Dim year As String = Convert.ToString(Date.Today.AddYears(-1).Year)
Dim nextyear As String = Convert.ToString(Date.Today.AddYears(15).Year)
Dim period As String
current = current.Substring(0, 5)
period = nextyear.Substring(2, 2)
cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
Dim current As Date = Date.Today
Dim year As Integer = Date.Today.AddYears(-1).Year
Dim nextyear As Integer = Date.Today.AddYears(15).Year
current = current.Substring(0, 5)
period = nextyear.Substring(2, 2)
For month = 1 To 12
    cmd = New OleDbCommand(sql, con, trans)
    cmd.Parameters.Add("@period", OleDbType.VarChar).Value = String.Format("{0:D2}{1}", month, period)
    cmd.Parameters.Add("@month", OleDbType.Integer).Value = month
    cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
    cmd.ExecuteNonQuery()
Next

Context

StackExchange Code Review Q#145940, answer score: 6

Revisions (0)

No revisions yet.