¿Alguien puede ofrecer comentarios sobre mi agente LotusScript?

Question

Hola, no soy desarrollador, por lo que desconozco las mejores prácticas. Creé esto para evitar la copia manual de datos de datos de registro. Este agente es para un feed único que copiaré y ajustaré para cada uno adicional. Para el feed especificado, lee el registro por última vez procesado y el número de archivos procesados ??hasta ahora hoy y ayer. También cuenta los archivos en la carpeta de entrada y lee la zona horaria del servidor. Cada elemento de datos está separado por una coma para csv y se envía por correo electrónico, que luego se aloja en un sitio web. Gracias por cualquier crítica constructiva.

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

Por no ser un desarrollador, puedes escribir mucho código :)

Si está buscando algunas lecciones para comenzar a convertirse en un buen desarrollador, tome el consejo de Mitch (de los comentarios) y divídalo en subrutinas. Lección 1: Definitivamente hay un código repetitivo aquí, y siempre es una buena idea poner código repetitivo en un método (función o subrutina) para que solo exista una vez. La sección que cuenta los archivos procesados ??y los archivos procesados ??anteriores parece similar y probablemente podría colocarse en una rutina como:

Function GetCountFilesProcessed() As Integer

    'code here

End Function

Sin embargo, incluso podría salvar la necesidad de eso si entiendo su código correctamente. En lugar de hacer ese ciclo extraño en el medio, parece que estás tratando de obtener simplemente un valor de una columna de una entrada de vista. Digamos que estás interesado en el valor de la columna 4. Simplemente puede obtener el valor de la columna que desea accediendo por índice. Por ejemplo, su variable de archivos podría establecerse directamente en el valor de la columna 4 mediante esta línea

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

De todos modos, la conclusión es que si este código funciona, ¡entonces tienes un buen comienzo!

Answer 2

En el lado del estilo, siempre me ha resultado más fácil mantener el código de otras personas cuando lo han hecho

1. Opción declarada 'explícita' en la sección Declaraciones
2. Variables declaradas de aproximadamente mayor a menor (por ejemplo, sesión antes de db antes de ver antes de doc)
3. Objetos de notas prefijadas con su tipo (docMail, dbMyDatabase, viewOutstandingInvoices)
4. Coloque todas las declaraciones en la parte superior antes (ayuda a encontrar la declaración cuando se encuentra con una variable)
5. Como otros han mencionado, divídalo en funciones / subs cuando corresponda.

Su comentario acerca de copiar este agente para otras instancias del mismo problema también levanta una bandera. Intente resolver lo que es común entre estos agentes e inserte esas funciones en una biblioteca de scripts. Este tipo de cosas ahorra mucho tiempo al mantener el código, ya que no necesita pensar de qué manera es diferente cada agente (por ejemplo, ¿mi cambio se aplica a todas las instancias de este agente, o solo a algunas de ellas?)

Answer 3

Desea descomponer su código mucho más y ... una pequeña cosa. En lugar de

while not item is nothing

que es una doble negación y un doblador de cerebro popular ... escriba:

do until item is nothing
  ...
loop

Esto también le permite salir del ciclo con exit do

Answer 4

puede optimizar sus dos bucles ForAll. Así es como se vería el primero:

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

Así es como se vería el segundo:

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

Answer 5

Solo una nota sobre este 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
    ....

Como dice Ken, puede obtener columnValues ??utilizando el método entry.ColumnValues ??(x), por lo que no es necesario interactuar con los valores. Pero; podrías haber hecho esto

    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

Algunos buenos puntos ya. Para agregarlos si tiene variables que comparten un objeto común, cree una clase. Agrega tus variables a esa clase.

Entonces, en lugar de decir:

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

Puedes tener:

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

y referencia:

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

La ventaja de esto es que puede agregar métodos para manipular esos datos y sabe que el código se relaciona con ese objeto en lugar de buscar en su aplicación.