7
\$\begingroup\$

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).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "02" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 2
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "03" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 3
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "04" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 4
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "05" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 5
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "06" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 6
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "07" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 7
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "08" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 8
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "09" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 9
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "10" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 10
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "11" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 11
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         cmd = New OleDbCommand(sql, con, trans)
         cmd.Parameters.Add("@period", OleDbType.VarChar).Value = "12" & period
         cmd.Parameters.Add("@month", OleDbType.Integer).Value = 12
         cmd.Parameters.Add("@year", OleDbType.Integer).Value = nextyear
         cmd.ExecuteNonQuery()

         trans.Commit()

     Else
     End If

 Catch ex As Exception
    trans.Rollback()
     silentErrorLog(ex)

  End Try
\$\endgroup\$
0

3 Answers 3

6
\$\begingroup\$
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.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you, this is exactly what I wanted! I considered a For Loop but wasn't sure how to create the period value in a parameter. My date formats are UK, dd/MM/yyyy, so current would get the dd/MM value (For the If clause), year would be the year just passed (2015, if it was 01/01/2016), and nextyear is a bit misleading, but 14 years worth are stored in the DB currently, so adding 15 years adds on the next set of years/periods. The period value is to get the yy part of the period, by substringing the current year. Could have done things differently, though, in hindsight. \$\endgroup\$
    – David
    Nov 3, 2016 at 9:23
3
\$\begingroup\$

The first thing I thought when I read your question is that the Period column should either be a computed column or not a column at all (with it being generated as necessary in stored procedures or select statements). The Period column will never be a value that cannot be easily computed from the other two columns. If it must be a column, why not make it a computed column with the formula (right('0'+cast([Month] as varchar(2)),2)+right([Year],2)) ?

Then the yearly update statement only needs to update the Year field for those 12 records that correspond to the previous year.

Edit: I see now this is MS Access, where I was thinking SQL Server 2000 for some reason. It still applies, but in Access it's called a calculated field, and the formula will need to change to something like FORMAT([Month],"00") & Right(String([Year]),2)

\$\endgroup\$
2
\$\begingroup\$

In VB.NET 14 (for VS2015) you can use String interpolation and make your code a lot cleaner (For Reference)

you could write the loop, that Mat's Mug talks about, like this:

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

With String Interpolation (in VB and C# at least) you can include your formatting in the variable call using the Colon just like you would in a string.format method call. This is really the exact same thing as using the String.Format except shorter.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.