Получить метод (ы) рабочей книги

Я переписываю свой модуль стандартных методов. Эти 3 используются для извлечения книг в качестве объектов рабочей книги. Стандартом, на который я нацелен, является «Сторонняя библиотека /надстройка». Итак:

  • Соответствуют ли эти функции принципу наименьшего удивления?
  • Я пропустил какие-либо потенциальные ошибки?

Как всегда, общая обратная связь также приветствуется.

Public Function GetWorkbook(ByVal strFileName As String, Optional ByVal strFilePath As Variant) As Workbook

        If Not (IsMissing(strFilePath) Or strFilePath.TypeName = "String") Then
            PrintErrorMessage "File Path was not supplied as a string", stopExecution:=True
        End If

    Dim wbIsOpen As Boolean

        wbIsOpen = IsWorkbookOpen(strFileName)
        If wbIsOpen Then
            Set GetWorkbook = Workbooks(strFileName)
        Else
            If Not IsMissing(strFilePath) Then
                Set GetWorkbook = OpenWorkbook(strFilePath & strFileName)
            Else
                PrintErrorMessage "Workbook (" & strFileName & ") is not open, and no filepath was supplied", stopExecution:=True
            End If
        End If

End Function
Public Function IsWorkbookOpen(ByVal strTargetName As String) As Boolean

    Dim wbTest As Workbook

        On Error Resume Next
            Set wbTest = Workbooks(strTargetName)
            IsWorkbookOpen = (wbTest.Name = strTargetName)
        On Error GoTo 0

End Function
Public Function OpenWorkbook(ByVal strFileName As String, strFilePath As String) As Workbook

    Dim strFullFilename As String
        strFullFilename = strFilePath & strFileName

    Dim wbOpenedSuccessfully As Boolean

        On Error Resume Next
            Set OpenWorkbook = Workbooks.Open(strFullFilename)
            wbOpenedSuccessfully = IsWorkbookOpen(strFileName)
        On Error GoTo 0

        If Not wbOpenedSuccessfully Then
            PrintErrorMessage "Failed to open workbook """ * strFullFilename & """", stopExecution:=True
        End If

End Function
11 голосов | спросил Kaz 16 WedEurope/Moscow2015-12-16T15:18:05+03:00Europe/Moscow12bEurope/MoscowWed, 16 Dec 2015 15:18:05 +0300 2015, 15:18:05

3 ответа


6

Почему strFilePath параметр optinal, если GetWorkbook не работает без пути? И если отсутствующий путь запускает ваш код stopExecution:=True, то зачем его проверять позже?

Public Function GetWorkbook(strFileName As String, strFilePath As String) As Workbook

  If IsWorkbookOpen(strFileName) Then
    Set GetWorkbook = Workbooks(strFileName)
  ElseIf strFilePath = "" Then
    PrintErrorMessage "File Path was not supplied as a string", stopExecution:=True
  Else
    Set GetWorkbook = OpenWorkbook(strFilePath, strFileName)
  End If

End Function
Public Function IsWorkbookOpen(ByVal strTargetName As String) As Boolean

  On Error Resume Next
    'If Len(Workbooks(strTargetName).Name) = 0 Then IsWorkbookOpen = False Else IsWorkbookOpen = True
    IsWorkbookOpen = (Len(Workbooks(strTargetName).Name) > 0)
    Err.Clear
  On Error GoTo 0

End Function
Public Function OpenWorkbook(ByVal strFileName As String, strFilePath As String) As Workbook

  On Error Resume Next
  Set OpenWorkbook = Workbooks.Open(strFilePath & strFileName)

  If Err.Number Or Not IsWorkbookOpen(strFileName) Then
    PrintErrorMessage "Failed to open workbook """ & strFullFilename & """", stopExecution:=True
  End If

  Err.Clear
  On Error GoTo 0

End Function

Я не являюсь другом объявления переменных, которые будут использоваться только один раз. Если это действительно необходимо, то лучше добавить комментарий к коду. Что делает wbIsOpen, wbTest, strFullFilename и wbOpenedSuccessfully устарели для моего глаза ...

И как всегда: старайтесь избегать On Error Resume Next как можно больше. Ошибки должны вызывать сообщение и не быть частью нормального поведения кодирования. Там много способов без ошибок получить эту информацию ...: /

Подсказка: на вашем OpenWorkbook есть опечатка, в сообщении об ошибке есть *, который, вероятно, должен быть &.

Просто чтобы подвести итоги ... делать это в одной функции, это вариант?

Public Function GetWorkbook(strFileName As String, Optional strFilePath As String) As Workbook

  On Error Resume Next
  If Len(Workbooks(strTargetName).Name) = 0 Then

    Err.Clear
    On Error Goto 0

    If Len(strFilePath) then
      Set GetWorkbook = OpenWorkbook(strFilePath & strFileName)
    End If

  Else

    On Error Goto 0
    Set GetWorkbook = Workbooks(strFileName)

  End If

End Function

Чтобы проверить, открыт ли wb, вы можете использовать Not GetWorkbook("workbookname") is Nothing, чтобы получить true, если он присутствует ...

Тем не менее мне не нравится использование ошибок и всегда предпочли бы что-то вроде этого: (он будет работать одинаково, но без ошибок)

Public Function GetWorkbookByName(strWorkbook As String, Optional strPath As String) As Workbook
  Dim MyWorkbook As Workbook

  For Each MyWorkbook In Workbooks
    If MyWorkbook.name = strWorkbook Then

      Set GetWorkbookByName = MyWorkbook
      Exit Function

    End If
  Next

  If Len(strPath) Then
    Set GetWorkbookByName = Workbooks.Open(strPath & strWorkbook)
  End If

End Function

Но, к сожалению, это не работает для более старых версий, и это просто не выглядит приятным для каждого процесса ...

ответил Dirk Reichel 16 WedEurope/Moscow2015-12-16T18:47:41+03:00Europe/Moscow12bEurope/MoscowWed, 16 Dec 2015 18:47:41 +0300 2015, 18:47:41
10

Я не буду упоминать снова, что я обнаружил, что outdenting из выражений Dim ухудшает читаемость. Вместо этого я сосредоточусь на этой части:

On Error Resume Next
    Set wbTest = Workbooks(strTargetName)
    IsWorkbookOpen = (wbTest.Name = strTargetName)
On Error GoTo 0

Какую ошибку вы ожидаете отсюда?

Workbooks(strTargetName)

Это приведет к появлению ошибки времени выполнения 9 - Подстрока за пределами диапазона . К тому времени вы уже знаете все, что вам нужно знать.

(wbTest.Name = strTargetName)

Это приведет к повышению ошибки времени выполнения 91 - переменной Object или с заблокированной переменной блока - но вы уже знаете, будет ли этот вызов работать, прежде чем вы даже попробуете его.

Это будет чище - немного переборщить с обработкой ошибок, но очиститель:

Public Function IsWorkbookOpen(ByVal wbFullFileName As String) As Boolean

    On Error GoTo CleanFail
    IsWorkbookOpen = Not Workbooks(wbFullFileName) Is Nothing

CleanExit:
    Exit Function
CleanFail:
    Err.Clear
    Resume CleanExit
End Function

Обратите внимание на разницу между strTargetName и wbFullFileName: «str» говорит «это строка». «wb» говорит «это что-то о книге». Оба венгерские, только один из них делает это правильно. Избегайте префиксов, указывающих тип.

ответил Mathieu Guindon 16 WedEurope/Moscow2015-12-16T18:01:03+03:00Europe/Moscow12bEurope/MoscowWed, 16 Dec 2015 18:01:03 +0300 2015, 18:01:03
8
Optional ByVal strFilePath As Variant

Почему вы разрешаете этому быть вариантом? Почему бы просто не потребовать тип строки (?). Это устраняет и без того неудобное сравнение If Not (IsMissing or). Вы можете выполнить проверку <> "" на вашем strFilePath вместо неловкости IsMissing.

Если вам почему-то нужно знать ситуацию, когда метод вызывается без аргумента vs пустой строки, то оставьте IsMissing проверьте, но я подозреваю, что вы в основном обеспокоены «прошел ли допустимый путь?»

Это позволяет использовать более чистую функцию.

Примечание. Я избавился от переменной wbIsOpen. Я также исправил ошибку компиляции в том, как вы вызываете OpenWorkbook (для этого требуются два параметра: у вас был & вместо ,)

Public Function GetWorkbook(ByVal strFileName As String, Optional ByVal strFilePath As String) As Workbook

    If IsWorkbookOpen(strFileName) Then
        Set GetWorkbook = Workbooks(strFileName)
    Else
        If strFilePath <> "" Then
            Set GetWorkbook = OpenWorkbook(strFilePath, strFileName)
        Else
            PrintErrorMessage "Workbook (" & strFileName & ") is not open, and no filepath was supplied", stopExecution:=True
        End If
    End If

End Function

Мне нравятся изменения, которые Мэтт предложил для IsWorkbookOpen, поэтому я не буду обращаться к этой функции.

Я бы предложил нечто подобное для OpenWorkbook. Я также очистил некоторые одноразовые переменные и сделал его лучше отступом (плюс исправление * to и amp; typo).

Public Function OpenWorkbook(ByVal strFileName As String, strFilePath As String) As Workbook

    On Error GoTo CleanFail
    Dim wbOpenedSuccessfully As Boolean

    Set OpenWorkbook = Workbooks.Open(strFilePath & strFileName)
    wbOpenedSuccessfully = IsWorkbookOpen(strFileName)

CleanExit:
    Exit Function
CleanFail:
    Err.Clear
    PrintErrorMessage "Failed to open workbook """ & strFullFilename & """", stopExecution:=True
    Resume CleanExit

End Function

Одна вещь, которую я могу сделать, это добавить «существует ли файл даже в нем?» проверьте здесь, поскольку это будет лучшей информацией для пользователя. Не зная полностью, что делает ваша обработка ошибок, добавив что-то вроде:

If Dir(strFilePath & strFileName) = "" Then
    'handle the "file does not exist" error
End If

предоставит пользователю дополнительную информацию о том, почему файл не открывается.


Я не знаю, что делает PrintErrorMessage. Но рассмотрите ситуацию, когда рабочая книга не открывается, ваш метод GetWorkbook не вернет объект рабочей книги. Но все ошибки обрабатываются на более низком уровне, поэтому вы не увидите ошибки в методе GetWorbook.

Возможно, ваш PrintErrorMessage обработает эту ситуацию (или ваш код при вызове GetWorkbook проверит, вернет ли действительный объект), но помните, что вы можете столкнуться с проблемами, возникающими в результате ошибок, возникающих в OpenWorkbook или IsWorkbookOpen.

Вы можете рассмотреть возможность передачи err.Description или err.Number в ваш метод PrintErrorMessage, поскольку они также могут быть полезны.

Другая потенциальная проблема, которую я вижу, заключается в том, что ваш пользователь передает путь к файлу без «\» в конце. Например:

GetWorkbook("myTest.xls", "C:\Temp") 

не удастся, потому что вы выполняете простую конкатенацию (вы попытаетесь открыть C: \ tempmyTest.xls). Не зная, как пользователи выбирают папку (они используют диалог выбора файлов?), Трудно порекомендовать надежное решение для этого, но простая проверка работоспособности может заключаться в том, чтобы проверить, что путь к файлу заканчивается символом \ и добавляет один, если не .

ответил Elysian Fields 16 WedEurope/Moscow2015-12-16T19:38:17+03:00Europe/Moscow12bEurope/MoscowWed, 16 Dec 2015 19:38:17 +0300 2015, 19:38:17

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132