Can anyone offer feedback on my LotusScript agent?

Question

Hi there I'm not a developer and so am unaware of best practices. I created this to bypass manual data copying of log data. This agent is for a single feed which I'll copy and adjust for each additional one. For the specified feed it reads the log for the last time processed and number of files processed so far today and for yesterday. It also counts files in the input folder and reads the server's timezone. Each data item is separated by a comma for csv and emailed, which later is hosted on a website. Thanks for any constructive criticism.

Sub Initialize
    Dim customername As String
    Dim servername As String
    Dim feedname As String
    Dim alertthresholdinhours As Integer
    Dim inputfeedpath As String

    ' Set for each feed
    customername = "gRrhio"
    servername = "gRrhioEdge2"
    feedname = "FF Thompson ADT"
    alertthresholdinhours = 6
    inputfeedpath = "\\mhinec\elycon\data\adt\*.*"

    ' Counts files in input folder
    Dim pathName As String, fileName As String
    Dim inputfeedcounter As Integer
    inputfeedcounter = 0
    pathName$ = inputfeedpath
    fileName$ = Dir$(pathName$, 0)
    Do While fileName$ <> ""
        inputfeedcounter = inputfeedcounter + 1
        fileName$ = Dir$()
    Loop

    Dim entry As NotesViewEntry   
    Dim vc As NotesViewEntryCollection
    Dim filesprocessed As Integer
    Dim session As New NotesSession
    Dim db As NotesDatabase
    Dim newDoc As NotesDocument
    Dim rtitem As NotesRichTextItem
    Set db = session.CurrentDatabase
    Dim view As NotesView
    Set view = db.GetView( "Sessions\by Feed" )
    Set newDoc = New NotesDocument( db )
    Set rtitem = New NotesRichTextItem( newDoc, "Body" )
    Dim todaysdate As New NotesDateTime("Today")
    Dim flag As Integer
    Dim counter As Integer
    Dim files As Integer
    Dim errors As Integer
    Dim lastdate As String
    Dim lastdayran As String
    Dim lasttime As String
    Dim lasttimeran As String
    Dim filesp As Integer
    Dim lastdayfiles As Integer
    Dim lastdaysfiles2 As Integer
    Dim terrors As Integer
    Dim lastdayerrors As Integer
    lastdate = ""
    lastdayran = ""
    counter = 0
    flag = 0
    filesp = 0
    lastdayfiles = 0
    lastdaysfiles2 = 0
    terrors = 0
    lastdayerrors = 0

    ' Finds date for last time processed, counts files processed and errors
    While flag = 0
        Dim dateTime As New NotesDateTime(todaysdate.DateOnly)
        Dim keyarray(1) As Variant
        keyarray(0) = feedname
        Set keyarray(1) = dateTime

        Set vc = view.GetAllEntriesByKey(keyarray, False)   
        Set entry = vc.GetFirstEntry

        If entry Is Nothing Then
            Call todaysdate.AdjustDay(-1)
        End If

        While Not entry Is Nothing
            files = 0
            Forall colval In entry.ColumnValues
                If counter = 9 Then
                    counter = 0
                Elseif counter = 8 Then
                    counter = 9
                Elseif counter = 7 Then
                    counter = 8
                Elseif counter = 6 Then
                    errors = Cint(colval)
                    counter = 7
                Elseif counter = 5 Then
                    counter = 6
                Elseif counter = 4 Then
                    files = Cint(colval)               
                    counter = 5
                Elseif counter = 3 Then
                    counter = 4
                Elseif counter = 2 Then
                    counter = 3
                    lasttime = colval
                Elseif counter = 1 Then
                    counter = 2
                    lastdate = colval
                Elseif counter = 0 Then
                    counter =  1
                End If           
            End Forall
            filesp = filesp + files
            terrors = terrors + errors
            Set entry=vc.GetNextEntry (entry)
            flag = 1
        Wend
    Wend
    lastdayfiles = filesp
    lastdayerrors = terrors
    lastdayran = lastdate
    lasttimeran = lasttime

    'Counts previous files processed
    filesp = 0
    terrors = 0
    lastdate = ""
    flag = 0
    Call todaysdate.AdjustDay(-1)   
    While flag = 0
        Dim dateTime2 As New NotesDateTime(todaysdate.DateOnly)
        Dim keyarray2(1) As Variant
        keyarray2(0) = feedname
        Set keyarray2(1) = dateTime2
        Set vc = view.GetAllEntriesByKey(keyarray2, False)   
        Set entry = vc.GetFirstEntry

        If entry Is Nothing Then
            Call todaysdate.AdjustDay(-1)
        End If

        While Not entry Is Nothing
            files = 0
            Forall colval In entry.ColumnValues
                If counter = 9 Then
                    counter = 0
                Elseif counter = 8 Then
                    counter = 9
                Elseif counter = 7 Then
                    counter = 8
                Elseif counter = 6 Then
                    counter = 7
                Elseif counter = 5 Then
                    counter = 6
                Elseif counter = 4 Then
                    files = Cint(colval)               
                    counter = 5
                Elseif counter = 3 Then
                    counter = 4
                Elseif counter = 2 Then
                    counter = 3
                Elseif counter = 1 Then
                    counter = 2
                Elseif counter = 0 Then
                    counter =  1
                End If           
            End Forall
            filesp = filesp + files
            Set entry=vc.GetNextEntry (entry)
            flag = 1
        Wend
    Wend
    lastdaysfiles2 = filesp

    ' Prints line of CSV into body of email
    Call rtitem.AppendText ( customername )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( servername )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( datetime.timezone )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( lastdayran )
    Call rtitem.AppendText ( " " )
    Call rtitem.AppendText ( lasttimeran )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( lastdayfiles )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( lastdayerrors )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( lastdaysfiles2 )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( inputfeedcounter )
    Call rtitem.AppendText ( ", " )
    Call rtitem.AppendText ( alertthresholdinhours )
    Call newDoc.Save( False, True )
    newDoc.Subject = feedname
    ' Running from server line should be
    'newDoc.SendTo = "Ecmon Feedcheck/Ecmonitor@ECMONITOR"
    newDoc.SendTo = "AX1Forward Feedcheck/ACHQ@company.com"
    newDoc.Send( False )
End Sub

Answer 1

Some good points already. To add to them if you have variables that share a common object then create a class. Add your variables to that class.

So instead of say:

Dim userFullName as String
Dim age as Integer
Dim addressLine1 as String
' ... etc.

You can have:

Class UserDetails 
  Dim fullName as String
  Dim age as Integer
  Dim addressLine1 as String
  ' ... etc
End Class

and reference:

Dim u as new UserDetails
u.fullName = "full name"
u.age = 22
u.addressLine1 = "1 main street"

The advantage of this is you can add methods to manipulate that data and you know the code relates to that object rather then hunting through your application.

Answer 2

you can optimize both of your ForAll loops. This is how the first one would look:

Forall colval In entry.ColumnValues
    Select Case (counter)
        Case 1: lastdate = colval
        Case 2: lasttime = colval
        Case 4: files = Cint(colval) 
        Case 6: errors = Cint(colval)
    End Select
    counter = (counter + 1) Mod 10    
End Forall

This is how the second would one would look like:

Forall colval In entry.ColumnValues
    if (counter = 4) Then files = Cint(colval)
    counter = (counter + 1) Mod 10
End Forall

Answer 3

You want to decompose your code much more and ... one little thing. Instead of

while not item is nothing

which is a double negation and a popular brain bender.. write:

do until item is nothing
  ...
loop

This also allows you to break out of the loop with exit do

Answer 4

On more of the style side of things, I've always found it easier to maintain other people's code when they've

1. Declared Option 'Explicit' in the Declarations section
2. Declared variables from approximately higher to lower (eg, session before db before view before doc)
3. Prefixed notes objects with their type (docMail, dbMyDatabase, viewOutstandingInvoices)
4. Put all of the Declarations at the top before (helps finding the declaration when you come across a variable)
5. As others have mentioned, break it up into functions/subs where applicable.

Your comment about copying this agent for other instances of the same problem also raises a flag. Try to work out what is common between these agents and push those functions into a script library. This sort of thing saves a lot of time when maintaining the code as you don't need to think about in what way each agent is different (Eg. does my change apply to all instances of this agent, or just some of them?)

Answer 5

Just a note regarding this bit

    While Not entry Is Nothing
        files = 0            
        Forall colval In entry.ColumnValues
            If counter = 9 Then
                counter = 0
            Elseif counter = 8 Then
                counter = 9
    ....

As ken says you can get columnValues by using the entry.ColumnValues(x) method so interating through the values is un-needed. But; you could have done this

    While Not entry Is Nothing
        files = 0            
        counter = 0
        Forall colval In entry.ColumnValues
            counter = counter + 1
            Select case counter
                case 6
                    errors = Cint(colval)
      .....
            end select

Answer 6

For not being a developer, you can write a lot of code :)

If you are looking for some lessons to start becoming a good developer, then take Mitch's advice (from the comments) and break up this into subroutines. Lesson 1: There's definitely some repetitve code here, and it's always a good idea to put repetitive code into a method (function or subroutine) so it only exists once. The section that counts files processed and previous files processed looks similar and could probably be put into a routine like:

Function GetCountFilesProcessed() As Integer

    'code here

End Function

However, you might even be able to save the need for that if I'm understanding your code correctly. Instead of doing that strange loop in the middle, it appears you're are trying to simply get a value from a column of a viewentry. Say you are interetsed in column 4's value. You can simply get at the value for the column you want by accessing it by index. For example, your files variable could be set directly to column 4's value by this line

files = Cint(entry.ColumnValues(4))  'check this, it might be 3 if the array is zero based.

Anyway, bottom line is if this code works then you're off to a good start!