Qualcuno può offrire feedback sul mio agente LotusScript?

Question

Ciao, non sono uno sviluppatore e quindi non sono a conoscenza delle migliori pratiche. Ho creato questo per bypassare la copia manuale dei dati di registro. Questo agente è per un singolo feed che copierò e aggiusterò per ogni ulteriore feed. Per il feed specificato legge il registro per l'ultima volta elaborato e il numero di file elaborati finora oggi e per ieri. Conta anche i file nella cartella di input e legge il fuso orario del server. Ogni elemento di dati è separato da una virgola per CSV e inviato via e-mail, che successivamente viene ospitato su un sito Web. Grazie per qualsiasi critica costruttiva.

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

Per non essere uno sviluppatore, puoi scrivere molto codice :)

Se stai cercando alcune lezioni per iniziare a diventare un buon sviluppatore, segui il consiglio di Mitch (dai commenti) e suddividilo in subroutine. Lezione 1: Qui c'è sicuramente del codice ripetitivo, ed è sempre una buona idea inserire il codice ripetitivo in un metodo (funzione o subroutine), quindi esiste solo una volta. La sezione che conta i file elaborati e i file precedenti elaborati sembra simile e probabilmente potrebbe essere inserita in una routine come:

Function GetCountFilesProcessed() As Integer

    'code here

End Function

Tuttavia, potresti anche essere in grado di salvare la necessità se capisco correttamente il tuo codice. Invece di fare quello strano loop nel mezzo, sembra che tu stia cercando di ottenere semplicemente un valore da una colonna di una viewentry. Supponi di essere intercettati nel valore della colonna 4. Puoi semplicemente ottenere il valore per la colonna desiderata accedendola per indice. Ad esempio, questa variabile di file potrebbe essere impostata direttamente sul valore della colonna 4 da questa riga

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

Comunque, la linea di fondo è se questo codice funziona, allora sei partito bene!

Answer 2

Per quanto riguarda lo stile, ho sempre trovato più facile mantenere il codice degli altri quando lo hanno

1. Opzione dichiarata 'Explicit' nella sezione Dichiarazioni
2. Variabili dichiarate da circa superiore a inferiore (ad esempio, sessione prima di db prima di vista prima di doc)
3. Oggetti note con prefisso con il loro tipo (docMail, dbMyDatabase, viewOutstandingInvoices)
4. Metti tutte le dichiarazioni in alto prima (aiuta a trovare la dichiarazione quando ti imbatti in una variabile)
5. Come altri hanno già detto, suddividilo in funzioni / sottotitoli ove applicabile.

Anche il tuo commento sulla copia di questo agente per altre istanze dello stesso problema solleva un flag. Prova a capire cosa c'è di comune tra questi agenti e inserisci queste funzioni in una libreria di script. Questo genere di cose consente di risparmiare molto tempo durante la manutenzione del codice in quanto non è necessario pensare in che modo ciascun agente è diverso (ad esempio, la mia modifica si applica a tutte le istanze di questo agente o solo ad alcuni di essi?)

Answer 3

Vuoi scomporre molto di più il tuo codice e ... una piccola cosa. Invece di

while not item is nothing

che è una doppia negazione e un popolare piegatore del cervello .. scrivi:

do until item is nothing
  ...
loop

Ciò consente anche di uscire dal ciclo con exit do

Answer 4

puoi ottimizzare entrambi i tuoi cicli ForAll. Ecco come apparirebbe il primo:

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

Ecco come apparirebbe il secondo:

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

Answer 5

Solo una nota relativa a questo 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
    ....

Come dice ken, puoi ottenere columnValues ??usando il metodo entry.ColumnValues ??(x), quindi l'interazione tra i valori non è necessaria. Ma; avresti potuto farlo

    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

Alcuni punti positivi già. Per aggiungerli se hai variabili che condividono un oggetto comune, crea una classe. Aggiungi le tue variabili a quella classe.

Quindi, invece di dire:

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

Puoi avere:

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

e riferimento:

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

Il vantaggio di questo è che puoi aggiungere metodi per manipolare quei dati e sai che il codice si riferisce a quell'oggetto piuttosto che cercare attraverso la tua applicazione.