1

I'm writing a sub in VBA that is trying to look at each element in one array and see if it shows up in another array. The first array is in rows A2:A325 in Sheet A, and the second array is over 250,000 values. I keep getting a runtime error 9: subscript out of range. My code is below

Private Sub ICD_DRG_Converter()

Dim StudyDRG() As Variant
Dim StudyICD10() As Variant
Dim element As String
Dim lLastRow, i, j, k As Long
Dim ICD10Code As String


Worksheets("Accepted DRG's").Activate

ReDim StudyDRG(1 To 325) As Variant

StudyDRG = Range("A2:A325") 'Populate the study DRG's into an array for comparison

Worksheets("full_appendix_B").Activate
lLastRow = ActiveSheet.Cells(Rows.Count, "B").End(xlUp).Row 'get the last row of data for sizing our ICD 10 array

ReDim StudyICD10(1 To (lLastRow)) As Variant

StudyICD10 = Range("B2:B" & lLastRow)

'i = 0
For i = LBound(StudyICD10) To UBound(StudyICD10)
    k = 1
    For j = LBound(StudyDRG) To UBound(StudyDRG)
        If StrComp(StudyICD10(i), StudyDRG(j), vbBinaryCompare) = 0 Then 'match between study DRG and ICD-10 DRG
            Worksheets("full_appendix_B").Activate
            ICD10Code = Range("A" & j).Value
            Worksheets("Accepted ICD-10").Activate
            Range("A" & k) = ICD10Code
            k = k + 1
            Exit For
        End If
    Next j
Next i





End Sub

The line that generates the error is:

If StrComp(StudyICD10(i), StudyDRG(j), vbBinaryCompare) = 0 Then

Any help on how to fix this would be appreciated. I've tried everything I know

Community
  • 1
  • 1
UWMoose77
  • 44
  • 7
  • strComp compares strings. You are feeding it variants of string type which are different. convert the variants to strings, like Cstr(StudyICD10(I)) – Variatus Mar 24 '17 at 03:57
  • with a quarter of a million entries to loop through for each entry to be evaluated you will have a lot of time for coffee while the computer runs hot. Consider using Find instead of the loop And compare. – Variatus Mar 24 '17 at 04:02
  • @Variatus I tried the Cstr(StudyICD10(I)) but unfortunately it still produces the same error. The idea of the loops is to take each item in the 250k array and look to see if it is in the 324 element array, then if it is, copy the data from a different cell in the row to a new sheet, after which I'll simply remove the duplicates. But yes, I have a feeling it will still take time. I'll try the Find here shortly. – UWMoose77 Mar 24 '17 at 04:37
  • I think @Rich Holton has the better idea, but mine might still apply. Make sure that both strings in StrComp are real strings. Going with Rich, the string you feed into StrComp might represent a Nullstring or even an error. Frankly, I think the wisest way to go from here is to abandon the present code in favour of a very pedestrian loop through Range("A2:A325") and Find to look for it among the 250,000 – Variatus Mar 24 '17 at 04:52

1 Answers1

1

When you use Range() to return a range of values into a variant array, the array is resized to match the range. So the results of

ReDim StudyDRG(1 To 325) As Variant
StudyDRG = Range("A2:A325") 

is that studyDRG will have elements from 1 to 324, not 1 to 325. Not only that, but Range() always returns a two dimensional array, even if there's only one column. So to refer to the element that corresponds to A2, you need to use StudyDRG(1,1), and A3 would be StudyDRG(1,2).

I hope this helps.

Rich Holton
  • 662
  • 5
  • 12
  • is there a way to simply make it a one dimensional array? I never seem to have had that problem before when reading a range to an array. I also initially had it at 324, but upped it in case something weird was happening to cause the subscript error. – UWMoose77 Mar 24 '17 at 04:40
  • See http://stackoverflow.com/questions/7649046/one-dimensional-array-from-excel-range, especially the up-voted answer that came after the accepted answer. – Rich Holton Mar 24 '17 at 04:50
  • Why do you want to create an array through which you can loop. Range(A2:A325) is already an array through which you can loop. – Variatus Mar 24 '17 at 04:55
  • @Rich Holton that up-voted answer would work for the smaller array, but would error out for the second array of 250k elements. I suppose I could just loop through that column and see if the value in each cell is in the smaller array. I'll try that tomorrow night and report back. – UWMoose77 Mar 24 '17 at 04:58
  • @Variatus I thought it would be faster to loop through an actual array instead of just a range. I could obviously be misunderstanding that concept though. I'm far from a computer scientist, and more of a data geek. This would be far easier with SQL, but I'm too lazy to set it up on my laptop. – UWMoose77 Mar 24 '17 at 05:00
  • Getting the data into an array and using that array is faster than using range references to the worksheet, especially if you're going to use the value more than once. Transposing once or twice, or copying from a 2D array to a 1D array may change the equation some. But you're thought was on track. – Rich Holton Mar 24 '17 at 05:14
  • Thanks @RichHolton. I actually just realized that what I needed was doable in PowerQuery after I mentioned the SQL thing to Variatus. I was able to accomplish what I needed in about 5 minutes with a Left Outer Join in PowerQuery. Thanks for all of the help and input! – UWMoose77 Mar 24 '17 at 05:35
  • @Rich Holton Thanks for the scientific input. In this case each of the 325 cells will be read only once. Therefore the time taken to read the array from the sheet into an array in memory, particularly a 1D array, shouldn't be too much different from that needed for a direct reference. But even if it is, this talk is about a tiny fraction of a second (for all 325 items) not comparable to the time it takes to look for each occurrence in a list of 250,000 values. – Variatus Mar 24 '17 at 09:00