ZeroEffect Posted July 31, 2006 Posted July 31, 2006 In my application I have timed events to play audio based on day of the week, start time and an end time. and the code below is how I have it running. I am looking for suggestions and examples of a better more effecient way, in short another set of eyes to look at it. I know it is ugly, it is from on of my first programs that I am updating/adding on to. Public Sub TodaysList(ByVal intAdvance As Integer, ByVal strDBFile As String) Dim Itm As ListViewItem Dim strConnect, strSql As String Dim St, Et, D, DS, AL, Dis As String Dim I As Integer Dim objDA1 As New OdbcDataAdapter Dim objDS1 As New System.Data.DataSet Dim dbCommand As New Data.OleDb.OleDbCommand Dim intCounter As Integer Dim mvarSysProjPath As String = Application.StartupPath & "\" strConnect = "Provider=MSDASQL/SQLServer ODBC;Driver={Microsoft Visual FoxPro Driver};" _ & "SourceType=DBF;SourceDB=" & mvarSysProjPath _ & ";InternetTimeout=300000;Transact Updates=True" strSql = "SELECT * FROM " & strDBFile & ".dbf" objDA1 = New OdbcDataAdapter(strSql, strConnect) objDA1.Fill(objDS1, strDBFile) Dim objDT1 As DataTable = objDS1.Tables(0) Dim blnDayNotFound As Boolean 'was the date found Dim strDay As String Do If strDBFile = "TMDENTS" Then MainForm.lvTodaysEvents.Items.Clear() 'clear the lv of events CurrentDayStr = Format(System.DateTime.FromOADate(Today.ToOADate + intAdvance), "dddd") 'format the date to the day of the week plus the advance number For I = 0 To objDS1.Tables(strDBFile).Rows.Count - 1 With objDS1.Tables(strDBFile).Rows(I) strDay = Replace(!Day, " ", "") If strDay = CurrentDayStr Then 'was the day found blnDayNotFound = True LVR = 1 ' As I am going through this can't remeber what this is for St = Replace(!StartTime, " ", "") Et = Replace(!EndTime, " ", "") D = Replace(!Day, " ", "") DS = RTrim(!Descript) Itm = MainForm.lvTodaysEvents.Items.Add(St) Itm.ImageIndex = 6 Itm.SubItems.Add(Et) Itm.SubItems.Add(D) Itm.SubItems.Add(DS) End If End With Next I ElseIf strDBFile = "PADEVENT" Then MainForm.lvTodayIboc.Items.Clear() CurrentDayStr = Format(System.DateTime.FromOADate(Today.ToOADate + intAdvance), "dddd") For I = 0 To objDS1.Tables(strDBFile).Rows.Count - 1 With objDS1.Tables(strDBFile).Rows(I) strDay = Replace(!Day, " ", "") If strDay = CurrentDayStr Then blnDayNotFound = True LVR = 1 St = Replace(!StartTime, " ", "") Et = Replace(!Day, " ", "") D = Replace(!Title, " ", "") DS = RTrim(!Artist) AL = RTrim(!Album) Dis = RTrim(!Discript) Itm = MainForm.lvTodayIboc.Items.Add(St) Itm.ImageIndex = 6 Itm.SubItems.Add(Et) Itm.SubItems.Add(Dis) Itm.SubItems.Add(D) Itm.SubItems.Add(DS) Itm.SubItems.Add(AL) End If End With Next I End If If blnDayNotFound = False Then intAdvance = intAdvance + 1 'add one to the date Else If LVR = 1 Then 'can't remember If intAdvance = 7 Then NextWeek = True End If NextInList(strDBFile) End If Exit Do End If Loop Until intAdvance > 7 objDS1.Dispose() End Sub 'Check the next event in the list 'and if it runs it runs another sub to set the data 'this sub is used with two different dbf files Public Sub NextInList(ByVal strDBFile As String) Dim TimeListDay As ListView.SelectedListViewItemCollection Dim SelectedTime As ListViewItem Dim strtime As Date Dim inttime As Integer Dim CurrentTimeStr As String Dim CurrentTimeInt As Integer Dim temp As Integer Dim strDay As String Dim intCount As Integer CurrentTimeStr = Replace(Format(Now(), "HH:mm"), ":", "") CurrentTimeInt = CurrentTimeStr CurrentDayStr = Format(System.DateTime.FromOADate(Today.ToOADate), "dddd") temp = 1 If strDBFile = "TMDENTS" Then TimeListDay = MainForm.lvTodaysEvents.SelectedItems intCount = MainForm.lvTodaysEvents.Items.Count ElseIf strDBFile = "PADEVENT" Then TimeListDay = MainForm.lvTodayIboc.SelectedItems intCount = MainForm.lvTodayIboc.Items.Count End If If NextWeek = False Then If intCount > 0 Then Do If strDBFile = "TMDENTS" Then MainForm.lvTodaysEvents.Items(temp - 1).Selected = True ElseIf strDBFile = "PADEVENT" Then MainForm.lvTodayIboc.Items(temp - 1).Selected = True End If For Each SelectedTime In TimeListDay strtime = SelectedTime.Text inttime = Replace(Format(strtime, "HH:mm"), ":", "") If strDBFile = "TMDENTS" Then strDay = SelectedTime.SubItems(2).Text ElseIf strDBFile = "PADEVENT" Then strDay = SelectedTime.SubItems(1).Text End If If inttime > CurrentTimeInt And strDay = CurrentDayStr Then If strDBFile = "TMDENTS" Then If blnRunTimedEvents = True Then RunTimedEvent(strDBFile) Exit Sub Else RunTimedEvent(strDBFile) End If ElseIf strDBFile = "PADEVENT" Then If blnRunIbocEvents = True Then If SelectedTime.Checked = False Then RunTimedEvent(strDBFile) Exit Sub ElseIf SelectedTime.Checked = True Then SelectedTime.Checked = True SelectedTime.ForeColor = System.Drawing.Color.Gray SelectedTime.Font = New Font(SelectedTime.Font, FontStyle.Strikeout) Exit For End If Else If SelectedTime.Checked = False Then RunTimedEvent(strDBFile) End If End If End If Exit Sub ElseIf inttime <= CurrentTimeInt And strDay = CurrentDayStr Or SelectedTime.Checked = True Then SelectedTime.Checked = True SelectedTime.ForeColor = System.Drawing.Color.Gray SelectedTime.Font = New Font(SelectedTime.Font, FontStyle.Strikeout) End If Next temp = temp + 1 Loop Until temp > intCount If strDay <> CurrentDayStr Then If strDBFile = "TMDENTS" Then MainForm.lvTodaysEvents.Items(0).Selected = True ElseIf strDBFile = "PADEVENT" Then MainForm.lvTodayIboc.Items(0).Selected = True End If 'Need to add event to trigger loading of single event that happened all ready End If For Each SelectedTime In TimeListDay If SelectedTime.Checked = True And SelectedTime.Index = (intCount - 1) Then TodaysList(1, strDBFile) RunTimedEvent(strDBFile) Exit Sub End If Next If intCount = 0 Then TodaysList(1, strDBFile) RunTimedEvent(strDBFile) Exit Sub End If End If Else NextWeek = False If strDBFile = "TMDENTS" Then MainForm.lvTodaysEvents.Items(0).Selected = True ElseIf strDBFile = "PADEVENT" Then MainForm.lvTodayIboc.Items(0).Selected = True End If End If End Sub Last part in another post ZeroEffect Quote If you can't find it, Build It. There is no place Like 127.0.0.1 also don't forget 1 + 1 = 10
ZeroEffect Posted July 31, 2006 Author Posted July 31, 2006 And the last part 'set the data 'Again one sub two dbf files Public Sub RunTimedEvent(ByVal strDBFile As String) Dim TimeListDay As ListView.SelectedListViewItemCollection Dim SelectedTime As ListViewItem If strDBFile = "TMDENTS" Then TimeListDay = MainForm.lvTodaysEvents.SelectedItems For Each SelectedTime In TimeListDay SE = "0" 'SE is second event/endtime strTimeEvent(0) = SelectedTime.Text 'start time strTimeEvent(1) = SelectedTime.SubItems(1).Text 'end time EventDay = SelectedTime.SubItems(2).Text If blnRunTimedEvents = True Then 'are timed events enabled? MainForm.TeLCD.SetAnimationOpts(1, 3, 150, True, False, -1) MainForm.TeLCD.SetAnimation(1, "Next Event " & SelectedTime.SubItems(2).Text & " [start] At " & strTimeEvent(0) & " [" & SelectedTime.SubItems(3).Text & "] ".PadRight(47)) Else MainForm.TeLCD.SetDisplay(1, "Timed Events Disabled") End If Next If MainForm.lvTodaysEvents.Items.Count > 0 Then SelectedTime.EnsureVisible() End If ElseIf strDBFile = "PADEVENT" Then TimeListDay = MainForm.lvTodayIboc.SelectedItems For Each SelectedTime In TimeListDay strTimeEvent(2) = SelectedTime.Text 'start time IbocEventDay = SelectedTime.SubItems(1).Text If blnRunIbocEvents = True Then MainForm.LCDIboc.SetAnimationOpts(1, 3, 150, True, False, -1) MainForm.LCDIboc.SetAnimation(1, "Next Pad Event On " & SelectedTime.SubItems(1).Text & " At " & strTimeEvent(2) & " [" & SelectedTime.SubItems(2).Text & "] ".PadRight(47)) Else MainForm.LCDIboc.SetDisplay(1, "IBOC PAD Events Disabled") End If Next If MainForm.lvTodayIboc.Items.Count > 0 Then SelectedTime.EnsureVisible() End If End If End Sub Again I know this is ugly that why I am look for ideas to tighten this up. Thanks for any help you may provide ZeroEffect Quote If you can't find it, Build It. There is no place Like 127.0.0.1 also don't forget 1 + 1 = 10
Administrators PlausiblyDamp Posted August 1, 2006 Administrators Posted August 1, 2006 Firstly, from a personal point of view, there is a lot of legacy VB6 style of coding in there (RTrim, Replace etc.) so you might want to convert them over to a more .Net way of doing things. If you prefer the VB syntax though that's fine. Secondly, again from a personal point of view, the naming convention(s) in use make for a fairly hard to read piece of code. You might want to pick a particular convention in regards to prefix or not, casing etc. and apply it as standard. Also your public methods are not following the MS guidelines andas such could also do with a revision. Some parts of the code appear to be very cut and paste i.e. If strDBFile = "TMDENTS" Then MainForm.lvTodaysEvents.Items.Clear() 'clear the lv of events CurrentDayStr = Format(System.DateTime.FromOADate(Today.ToOADate + intAdvance), "dddd") 'format the date to the day of the week plus the advance number For I = 0 To objDS1.Tables(strDBFile).Rows.Count - 1 With objDS1.Tables(strDBFile).Rows(I) strDay = Replace(!Day, " ", "") If strDay = CurrentDayStr Then 'was the day found blnDayNotFound = True LVR = 1 ' As I am going through this can't remeber what this is for St = Replace(!StartTime, " ", "") Et = Replace(!EndTime, " ", "") D = Replace(!Day, " ", "") DS = RTrim(!Descript) Itm = MainForm.lvTodaysEvents.Items.Add(St) Itm.ImageIndex = 6 Itm.SubItems.Add(Et) Itm.SubItems.Add(D) Itm.SubItems.Add(DS) End If End With Next I ElseIf strDBFile = "PADEVENT" Then MainForm.lvTodayIboc.Items.Clear() CurrentDayStr = Format(System.DateTime.FromOADate(Today.ToOADate + intAdvance), "dddd") For I = 0 To objDS1.Tables(strDBFile).Rows.Count - 1 With objDS1.Tables(strDBFile).Rows(I) strDay = Replace(!Day, " ", "") If strDay = CurrentDayStr Then blnDayNotFound = True LVR = 1 St = Replace(!StartTime, " ", "") Et = Replace(!Day, " ", "") D = Replace(!Title, " ", "") DS = RTrim(!Artist) AL = RTrim(!Album) Dis = RTrim(!Discript) Itm = MainForm.lvTodayIboc.Items.Add(St) Itm.ImageIndex = 6 Itm.SubItems.Add(Et) Itm.SubItems.Add(Dis) Itm.SubItems.Add(D) Itm.SubItems.Add(DS) Itm.SubItems.Add(AL) End If End With Next I End If could probably be replace with a method... Public Sub Test(ByVal lv As ListView, ByVal padEvent As Boolean) Dim Itm As ListViewItem Dim strConnect, strSql As String Dim St, Et, D, DS, AL, Dis As String Dim I As Integer Dim objDT1 As DataTable = objDS1.Tables(0) Dim blnDayNotFound As Boolean 'was the date found Dim strDay As String Dim objDA1 As New OdbcDataAdapter Dim objDS1 As New System.Data.DataSet Dim dbCommand As New Data.OleDb.OleDbCommand Dim intCounter As Integer Dim mvarSysProjPath As String = Application.StartupPath & "" strConnect = "Provider=MSDASQL/SQLServer ODBC;Driver={Microsoft Visual FoxPro Driver};" _ & "SourceType=DBF;SourceDB=" & mvarSysProjPath _ & ";InternetTimeout=300000;Transact Updates=True" strSql = "SELECT * FROM " & strDBFile & ".dbf" objDA1 = New OdbcDataAdapter(strSql, strConnect) objDA1.Fill(objDS1, strDBFile) lv.Items.Clear() 'clear the lv of events CurrentDayStr = Format(System.DateTime.FromOADate(Today.ToOADate + intAdvance), "dddd") 'format the date to the day of the week plus the advance number For I = 0 To objDS1.Tables(strDBFile).Rows.Count - 1 With objDS1.Tables(strDBFile).Rows(I) strDay = Replace(!Day, " ", "") If strDay = CurrentDayStr Then 'was the day found blnDayNotFound = True LVR = 1 ' As I am going through this can't remeber what this is for St = Replace(!StartTime, " ", "") Et = Replace(!EndTime, " ", "") D = Replace(!Day, " ", "") DS = RTrim(!Descript) Itm = lv.Items.Add(St) Itm.ImageIndex = 6 If padEvent Then Itm.SubItems.Add(Et) Itm.SubItems.Add(Dis) Itm.SubItems.Add(D) Itm.SubItems.Add(DS) Itm.SubItems.Add(AL) Else Itm.SubItems.Add(Et) Itm.SubItems.Add(D) Itm.SubItems.Add(DS) End If End If End With Next I End Sub Public Sub TodaysList(ByVal intAdvance As Integer, ByVal strDBFile As String) Do If strDBFile = "TMDENTS" Then Test(MainForm.lvTodaysEvents, False) ElseIf strDBFile = "PADEVENT" Then Test(MainForm.lvTodayIboc, True) End If If blnDayNotFound = False Then intAdvance = intAdvance + 1 'add one to the date Else If LVR = 1 Then 'can't remember If intAdvance = 7 Then NextWeek = True End If NextInList(strDBFile) End If Exit Do End If Loop Until intAdvance > 7 End Sub although a more appropriate method name than 'Test' is in order ;) Once you make a start you are bound to find out other things that can be improved etc. Ideally if you have a set of test cases (unit tests are ideal) then as you tidy up the code you can always regression test it to prevent new bugs being introduced. Quote Posting Guidelines FAQ Post Formatting Intellectuals solve problems; geniuses prevent them. -- Albert Einstein
ZeroEffect Posted August 1, 2006 Author Posted August 1, 2006 Thanks PlausiblyDamp :) Yes, this was from a vb6 app Yes, some of it was cut and paste. Not a steal cut and paste but trial and error cut and paste I didn't know I could do that with a listview or any other control, I'll have to look into methods I'll be going through the code I have and do some cleaning up. Do you think this is a good way to build a list of timed events? Thanks ZeroEffect Quote If you can't find it, Build It. There is no place Like 127.0.0.1 also don't forget 1 + 1 = 10
Administrators PlausiblyDamp Posted August 1, 2006 Administrators Posted August 1, 2006 I have very little knowledge of working with FoxPro - however if it supports the concept of a SELECT ... WHERE statement then it will probably be far more efficient to use that rather than manually looping through the records yourself. Quote Posting Guidelines FAQ Post Formatting Intellectuals solve problems; geniuses prevent them. -- Albert Einstein
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.