Прежде всего, спасибо за тестирование вашего кода VBA.Профессиональные разработчики на каждом языке пишут юнит-тесты, и с Rubberduck (отказ от ответственности: я управляю этим проектом) вы активизируете свою игру и вносите свой вклад, чтобы сделать VBA менее страшным языком .
Однако не весь код поддается тестированию.Чтобы написать модульные тесты для функции, эта функция должна быть написана таким образом, чтобы связывание сводилось к минимуму, а ее зависимости в идеале воспринимались как параметры.
One Thing , который определенно делает функцию непроверяемой, - это когда эта функция включает взаимодействие с пользователем.MsgBox открывает модальное окно, которое необходимо закрыть вручную, поэтому тестируемый код избегает его 1 .Stop - это код отладчика, который не должен быть в рабочем состоянии, а также препятствует выполнению теста.
Вы попали в автобус или отправляетесь для решения новых задач в другом месте, икто-то должен завтра принять этот код.Будут ли они проклинать ваше имя или хвалить вашу работу?
Я не могу прочитать TPR_TNR_FPR_FNR и сразу же выяснить, что он делает, просто по имени.Это проблема, потому что это делает обслуживание намного сложнее, чем нужно: если мы не знаем, что функция должна делать, как мы узнаем, что она делает это правильно?С набором тестов с хорошими именами мы можем знать, как он ведет себя во всех случаях ... при условии использования тестов с хорошими именами.Test1 мало что нам говорит, кроме хорошо, он что-то тестирует .
Сначала отбросьте операторы MsgBox и Stop - вместо этого выведите ошибку в этом предложении guard:
If WorksheetFunction.CountA(expected_vals) <> WorksheetFunction.CountA(pred_vals) Then
Err.Raise 5, "TPR_TNR_FPR_FNR", "Cells in Expected_vals and pred_vals must be the same in length"
End If
Обратите внимание, что здесь не сравнивается количество строк и / или столбцов каждого диапазона; только то, что у них одинаковое количество непустых ячеек .Только с этим одним оператором Err.Raise я могу написать несколько модульных тестов:
- При заданных диапазонах одинакового размера с одинаковым количеством непустых ячеек ошибка не выдается.
- При наличии диапазонов одинакового размера с разным количеством непустых ячеек выдается ошибка 5.
- При наличии диапазонов разного размера с одинаковым количеством непустых ячеек не выдается ошибка.
- При наличии диапазонов разного размера с разным количеством непустых ячеек выдается ошибка 5.
- При наличии несмежных диапазонов с одинаковым количеством непустых ячеек ошибка не выдается.
- При наличии двух диапазонов без непустых ячеек ошибка не выдается.
Если какое-либо из этих утверждений выглядит неправильно, ваш код работает не так, как задумано -потому что все эти тесты пройдут, учитывая, что выдается ошибка, когда WorksheetFunction.CountA возвращает различное значение для двух диапазонов.
Пройдя предложение guard, функция переходит к итерации ячеек в expected_vals, что имеетсопоставление значенийпараметр val_tested.
Функция работает с Range объектами, перебирает ячейки, неявно сравнивая Range.[_Default] (Value) со значением Integer: если какая-либо из ячеек в expected_vals содержит ошибку, Несоответствие типов здесь выдается ошибка:
If cell = val_tested Then
Поскольку вышеизложенное действительно делает это:
If cell.Value = val_tested Then
Range.Value являетсяVariant, который может содержать любое значение: числовые значения равны Variant/Double, поэтому даже в «счастливом пути» происходит неявное преобразование, чтобы сравнить это Double с предоставленным Integer.Похоже, что val_tested должно быть Double.
Но Range.Value также может быть Variant/Error, и этот вариантный подтип не может сравниваться с любым другим типом без выброса несоответствия типов.Если ожидается выбрасывание этого несоответствия типов, для этого должен быть тест.Иначе, это должно быть обработано - и тогда должен быть тест для этого:
- Учитывая значение ошибки в
expected_vals, выдает ошибку 13 (или нет?)
Если этой ошибки не должно быть, функция должна активно ее предотвратить:
For Each cell In expected_vals
If Not IsError(cell.Value) Then
If cell.Value = val_tested Then count_all = count_all + 1
End If
Next
Таким образом, count_all - это на самом деле количество ячеек в expected_vals, которые имеют значение, которое соответствует предоставленному параметру val_tested: я считаю, что matchingExpectedValuesCount будет более описательным / значимым именем для него, и оно должно бытьобъявлено локально с помощью оператора Dim (инспекции Rubberduck должны предупредить вас об этом ... и еще пару вещей).
Далее у нас есть цикл For, который делает удивительное предположение:
For i = 1 To expected_vals.Cells.Count
If (expected_vals.Cells(i).Value = pred_vals.Cells(i).Value) And (expected_vals.Cells(i).Value = val_tested) Then
Теперь мы предполагаем очень специфическую форму для предоставленных диапазонов.Если мы добрались до этого с диапазоном в 2 столбца или несмежным диапазоном нескольких областей, это то место, где мы собираемся взорвать.
Оговорка о защите должна защищать от этого предположения,и выбросить ошибку соответственно.WorksheetFunction.CountA / количество непустых ячеек в каждом заданном диапазоне недостаточно для правильной защиты от неверных входных данных.Примерно так должно быть более точно:
If expected_vals.Rows.Count <> pred_vals.Rows.Count _
Or expected_vals.Columns.Count <> 1 _
Or pred_vals.Columns.Count <> 1 _
Then
Err.Raise 5, "TPR_TNR_FPR_FNR", "Invalid inputs"
End If
Теперь предположения будут такими:
- При заданных диапазонах одинакового размера с одинаковым количеством ячеек ошибка не выдается.
- При заданных диапазонах одинакового размера с разным количеством ячеек выдается ошибка 5.
- При заданных диапазонах разного размера с одинаковым количеством ячеек выдается ошибка 5.
- Данодиапазоны разных размеров с разным количеством ячеек, выдается ошибка 5.
- При наличии несмежных диапазонов с одинаковым количеством непустых ячеек выдается ошибка 5.
- с учетом двух диапазоновбез каких-либо непустых ячеек ошибка не выдается.
Теперь, когда это установлено, 2-й цикл также должен обрабатывать Variant/Error для предотвращения несоответствия типов ошибок.
If Not IsError(expected_vals.Cells(i).Value) _
And Not IsError(pred_vals.Cells(i).Value) _
Then
If (expected_vals.Cells(i).Value = pred_vals.Cells(i).Value) And (expected_vals.Cells(i).Value = val_tested) Then
count_correct = count_correct + 1
End If
End If
Наконец, присвоение результата функции приведет к ошибке деления на ноль, если count_all равно 0:
TPR_TNR_FPR_FNR = count_correct / count_all
Если это ожидается, должен быть проверенЭто.В противном случае оно должно быть защищено, суррогатное значение должно быть возвращено (например, -1 или 0), ... и для него должен быть тест!
- При отсутствии ячеек в
expected_vals соответствует предоставленному val_tested значению, выдается ошибка 11.
или ..
- При отсутствии ячеек в
expected_vals соответствует предоставленному val_tested значению,возвращает 0.
Написание тестов
Для каждого отдельного маркера "Given ..., ..." выше, тест должен быть написан, чтобы доказать это.В вашем тесте есть ряд уже выявленных проблем , а также ряд неопознанных проблем.
Тайный соус для написания хороших тестов - управление входными данными.Наличие параметров Excel.Range делает его сложнее, чем необходимо: теперь вам нужно иметь тестовый лист с фактическим диапазоном тестов с кучей тестовых значений, ... и это кошмар, потому что теперь от того, пройдут тесты или нет, зависит отвещи, которых нет в самих тестах - и это очень плохо: хорошие тесты должны иметь надежные, воспроизводимые, согласованные результаты.
В этой функции я не видел ничего такого, что говорит о том, что нужно для работы с Range параметрами.На самом деле, работа с простыми массивами сделает его значительно более эффективным и намного проще утверждать предположения в предложении guard - просто проверьте границы массива!Работа с простыми массивами также означает, что тесты теперь могут быть автономными: код установки теста может легко определять тестовые массивы для предоставления функции, тем более что мы установили, что эти массивы должны быть одномерными.
Таким образом, необходимо переписать функцию для работы с массивами Variant.
Как только это будет сделано (я оставлю эту часть вам!), Вы можете легко настроить все необходимые входные данные для всех тестов,и тестовые шаблоны Rubberduck делают это довольно легко.Вот как может выглядеть один из этих тестов:
'@TestMethod
Public Sub GivenDifferentSizeArrays_Throws()
Const ExpectedError As Long = 5
On Error GoTo TestFail
'Arrange:
Dim expectedValues As Variant
expectedValues = Array(1, 2, 3)
Dim predValues As Variant
predValues = Array(1, 2, 3, 4)
'Act:
Dim result As Double
result = TPR_TNR_FPR_FNR(expectedValues, predValues, 1)
Assert:
Assert.Fail "Expected error was not raised."
TestExit:
Exit Sub
TestFail:
If Err.Number = ExpectedError Then
Resume TestExit
Else
Resume Assert
End If
End Sub
В этом тесте (обратите внимание, что для изменения функции требуется два варианта массива, а не Range параметров), ожидается, что ошибка 5 будет вызванавызов функции, учитывая два массива разного размера: если ожидаемая ошибка не возникает, тест не пройден.Если это так, тест проходит успешно.
Другой тест может подтвердить, что выдается ошибка 13 с учетом значения ошибки в одной из ячеек - здесь значение #N/A ошибка ячейки:
'Arrange:
Dim expectedValues As Variant
expectedValues = Array(1, 2, 3)
Dim predValues As Variant
predValues = Array(CVErr(xlErrNA), 2, 3)
И так далее, пока все мыслимые крайние случаи не будутохвачено: если все ваши тесты имеют осмысленное имя, вы можете точно знать, как ваша функция должна работать, просто прочитав названия тестов в проводнике тестов Rubberduck и одним щелчком мыши запустите весь пакет, увидев, что все они становятся зелеными, доказывая, что функция работает точно так, как задумано - даже после того, как вы внесли в нее изменения.
Явные предположения
Вот переписанная версия вашей функции, которая делает ее предположения явными идолжно быть намного проще писать тесты:
Public Function TPR_TNR_FPR_FNR(ByRef expected_vals As Variant, ByRef pred_vals As Variant, ByVal val_tested As Double) As Double
Dim workValues As Variant
Dim predValues As Variant
If Not IsArray(expected_vals) Or Not IsArray(pred_vals) Then
Err.Raise 5, "TPR_TNR_FPR_FNR", "Parameters must be arrays."
Else
workValues = expected_vals
predValues = pred_vals
End If
If TypeOf expected_vals Is Excel.Range Then
If expected_vals.Columns.Count <> 1 Then Err.Raise 5, "TPR_TNR_FPR_FNR", "'expected_vals' must be a single column."
workValues = Application.WorksheetFunction.Transpose(expected_vals)
End If
If TypeOf pred_vals Is Excel.Range Then
If pred_vals.Columns.Count <> 1 Then Err.Raise 5, "TPR_TNR_FPR_FNR", "'pred_vals' must be a single column."
predValues = Application.WorksheetFunction.Transpose(pred_vals)
End If
If UBound(workValues) <> UBound(predValues) Then
Err.Raise 5, "TPR_TNR_FPR_FNR", "'expected_vals' and 'pred_vals' must be the same size."
End If
Dim matchingExpectedValuesCount As Long
Dim currentIndex As Long
For currentIndex = LBound(workValues) To UBound(workValues)
If workValues(currentIndex) = val_tested Then
matchingExpectedValuesCount = matchingExpectedValuesCount + 1
End If
Next
If matchingExpectedValuesCount = 0 Then
TPR_TNR_FPR_FNR = 0
Exit Function
End If
Dim count_correct As Long
For currentIndex = LBound(predValues) To UBound(predValues)
If workValues(currentIndex) = predValues(currentIndex) And workValues(currentIndex) = val_tested Then
count_correct = count_correct + 1
End If
Next
TPR_TNR_FPR_FNR = count_correct / matchingExpectedValuesCount
End Function
Обратите внимание, что я не на 100% ясен в отношении цели, поэтому я оставил несколько идентификаторов, как у вас, - я быТем не менее, настоятельно рекомендуем переименовать их.
1 Функции модульного тестирования Rubberduck включают "подделки" API, который позволяет вам настраивать тест и буквально перехватывать вызовы MsgBox (и несколько других), позволяя вам написать тест для процедуры, которая обычно выдает окно сообщения , даже не отображая его во время выполнения теста .API также позволяет настроить его возвращаемое значение, так что вы можете, например, проверить, что происходит, когда пользователь нажимает «Да», а затем другой тест может подтвердить, что происходит, когда пользователь нажимает «Нет».