Может ли кто-нибудь дать отзыв о моем агенте LotusScript?

Привет, я не разработчик и поэтому не знаком с лучшими практиками. Я создал это, чтобы обойти ручное копирование данных журнала. Этот агент предназначен для одного канала, который я буду копировать и настраивать для каждого дополнительного. Для указанного фида он считывает журнал последних обработок и количество файлов, обработанных за сегодня и за вчерашний день. Он также считает файлы во входной папке и считывает часовой пояс сервера. Каждый элемент данных отделяется запятой для CSV и отправляется по электронной почте, которая позже размещается на веб-сайте. Спасибо за конструктивную критику.

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/[email protected]"
    newDoc.Send( False )
End Sub

person Todd    schedule 15.03.2009    source источник
comment
О попробовал бы для начала разбить часть вашей логики на подпрограммы ....   -  person Mitch Wheat    schedule 15.03.2009


Ответы (6)


Не будучи разработчиком, можно написать много кода :)

Если вы хотите извлечь уроки, чтобы стать хорошим разработчиком, прислушайтесь к совету Митча (из комментариев) и разбейте его на подпрограммы. Урок 1. Здесь определенно есть повторяющийся код, и всегда полезно поместить повторяющийся код в метод (функцию или подпрограмму), чтобы он существовал только один раз. Раздел, в котором подсчитываются обработанные файлы и предыдущие обработанные файлы, выглядит примерно одинаково и, вероятно, может быть помещен в такую ​​процедуру, как:

Function GetCountFilesProcessed() As Integer

    'code here

End Function

Однако вы можете даже избавиться от этого, если я правильно понимаю ваш код. Вместо того, чтобы делать этот странный цикл посередине, похоже, вы пытаетесь просто получить значение из столбца окна просмотра. Допустим, вас интересует значение столбца 4. Вы можете просто получить значение для нужного столбца, открыв его по индексу. Например, переменная ваших файлов может быть установлена ​​непосредственно в значение столбца 4 в этой строке.

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

В любом случае, суть в том, что если этот код работает, значит, у вас хороший старт!

person Ken Pespisa    schedule 16.03.2009
comment
Я посмотрю на предложение значений столбца. Спасибо за вклад! - person Todd; 17.03.2009

Что касается стиля, то мне всегда было легче поддерживать код других людей, когда они

  1. Заявлено Option 'Explicit' в разделе "Объявления".
  2. Объявленные переменные примерно от большего к меньшему (например, сеанс перед db перед просмотром перед документом)
  3. Объекты заметок с префиксом с указанием их типа (docMail, dbMyDatabase, viewOutstandingInvoices)
  4. Поместите все объявления вверху перед (помогает найти объявление, когда вы сталкиваетесь с переменной)
  5. Как уже упоминалось, разбейте его на функции / подпрограммы, где это применимо.

Ваш комментарий о копировании этого агента для других экземпляров той же проблемы также поднимает флаг. Попытайтесь выяснить, что общего между этими агентами, и поместить эти функции в библиотеку сценариев. Подобные вещи экономят много времени при сопровождении кода, поскольку вам не нужно думать о том, чем каждый агент отличается (например, мои изменения применяются ко всем экземплярам этого агента или только к некоторым из них?)

person Jon McAuliffe    schedule 14.05.2009
comment
Вау, спасибо. Я не слышал об этом. Библиотека скриптов звучит интригующе. Круто, это просто вдохновило меня на лучший способ делать вещи! - person Todd; 21.05.2009

Вы хотите еще больше разложить свой код и ... одну мелочь. Вместо

while not item is nothing

что является двойным отрицанием и популярным изгибом мозгов ... напишите:

do until item is nothing
  ...
loop

Это также позволяет выйти из цикла с помощью exit do.

person stwissel    schedule 30.06.2009
comment
Спасибо, я ценю вашу помощь! - person Todd; 04.07.2009

вы можете оптимизировать оба цикла ForAll. Вот как будет выглядеть первый:

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

Вот так бы выглядело второе:

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

Просто примечание об этом бите

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

Как говорит Кен, вы можете получить columnValues ​​с помощью метода entry.ColumnValues ​​(x), поэтому взаимодействие через значения не требуется. Но; ты мог бы сделать это

    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
person Martlark    schedule 18.03.2009

Уже есть несколько хороших моментов. Чтобы добавить к ним, если у вас есть переменные, которые имеют общий объект, создайте класс. Добавьте свои переменные в этот класс.

Поэтому вместо того, чтобы сказать:

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

Вы можете иметь:

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

и ссылка:

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

Преимущество этого заключается в том, что вы можете добавлять методы для управления этими данными, и вы знаете, что код относится к этому объекту, а не к поиску через ваше приложение.

person Sio    schedule 25.02.2010