2

I want to preface this by saying I have no idea whatsoever why my code is doing what it is doing. I'm really hoping one of the VBA gurus on here can help. Also, this is my first post so I did my best to follow the rules, but if I did anything wrong please point it out.


I have a sub that iterates through a column of data and creates an array. It calls a function that checks to see if the particular value is already in the array. If it is not, then the array is re-dimensioned, the value is inserted, and the process begins again, continuing until the end of the list is reached. I end up with an array totaling 41 values, but 4 of them have been duplicated twice, so there are only 37 unique values in the array.

I can't for the life of me figure out what sets these values apart or why they are being duplicated. The total list is 700+ values long so I figured I should see other values duplicated, but I'm not.

Here is the code for the sub that creates the array:

Sub ProductNumberArray(strWrkShtName As String, strFindColumn As String, blAsGrp As Boolean, iStart As Integer)
    Dim i As Integer
    Dim lastRow As Integer
    Dim iFindColumn As Integer
    Dim checkString As String

    With wbCurrent.Worksheets(strWrkShtName)
        iFindColumn = .UsedRange.Find(strFindColumn, .Range("A1"), xlValues, xlWhole, xlByColumns).Column
        lastRow = .Cells(Rows.Count, iFindColumn).End(xlUp).row
        For i = iStart To lastRow
            checkString = .Cells(i, iFindColumn).Value
            If IsInArray(checkString, arrProductNumber) = False Then
                If blAsGrp = False Then
                    ReDim Preserve arrProductNumber(0 To j)
                    arrProductNumber(j) = checkString
                    j = j + 1
                Else
                    ReDim Preserve arrProductNumber(1, 0 To j)
                    arrProductNumber(0, j) = .Cells(i, iFindColumn - 1).Value
                    arrProductNumber(1, j) = checkString
                    j = j + 1
                End If
            End If
        Next i
    End With
End Sub

And here is the code that checks to see if the checkString value is in the array:

Function IsInArray(stringToBeFound As String, arr As Variant) As Boolean
    Dim bDimen As Byte, i As Long

    On Error Resume Next
    If IsError(UBound(arr, 2)) Then bDimen = 1 Else bDimen = 2
    On Error GoTo 0

    Select Case bDimen
    Case 1
        On Error Resume Next
        IsInArray = Application.Match(stringToBeFound, arr, 0)
        On Error GoTo 0
    Case 2
        For i = 1 To UBound(arr, 2)
            On Error Resume Next
            IsInArray = Application.Match(stringToBeFound, Application.Index(arr, , i), 0)
            On Error GoTo 0
            If IsInArray = True Then Exit For
        Next
    End Select
End Function

Any help at all will be most welcome. I've been able to find answers to all my questions previously (or at least debug and see an obvious problem) but this one has stumped me. I hope someone can figure out what is going on.


[EDIT] Here is the code where the sub is being called:

Sub UpdatePSI()    
    Set wbCurrent = Application.ActiveWorkbook
    Set wsCurrent = wbCurrent.ActiveSheet

    frmWorkbookSelect.Show

    If blFrmClose = True Then 'if the user closes the selection form, the sub is exited
        blFrmClose = False
        Exit Sub
    End If

    Set wsSelect = wbSelect.Sheets(1)

    Call ProductNumberArray("Forecast", "Item", True, 3)

wbCurrent, wsCurrent, and blFrmClose are defined in the general declarations.

6
  • Can you add some code to show how the first sub is being called? wbCurrent doesn't seem to be defined anywhere. And why is the array being redimensioned if the value is found. Why not just not add the value if it already exists? Commented Sep 27, 2017 at 17:34
  • 2
    You should look into using a Scripting Dictionary instead, as it has an .Exists method you can use to easily create a distinct list. It can then later be turned into an Array Commented Sep 27, 2017 at 17:35
  • @braX In some instances I need 2 parts to my array, in others only 1. I've done it this way so I can reuse my array sub in multiple places. From what I can tell Scripting Dictionaries don't allow me to do that. @SJR If the value is not found then the function returns False and the value is added. If the value is found then the function returns True and the If statement is skipped. Commented Sep 27, 2017 at 17:48
  • @PrimeTurtler Sure it can. With just the two items, you could use the Key and Value parameters; or you could store both parts as the Value in an array. Commented Sep 27, 2017 at 17:55
  • @RonRosenfeld I was under the impression I couldn't do that, though now I have no idea where I got that idea from. I'll try it and post an update. Commented Sep 27, 2017 at 18:07

4 Answers 4

1

None of the (wild) guesses so far as to what's causing the duplicate issue you're having are even close. It is actually caused by a bug in your code.

In your IsInArray function, you finish the array loop index at the wrong value. For i = 1 To UBound(arr, 2) should be For i = 1 To UBound(arr, 2) - LBound(arr, 2) + 1. As your index finishes one short, this means that the compare string is never checked against the last array item and as a consequence, the second of any consecutive identical values will be copied across as a duplicate. Always use both LBound and UBound in the index parameters to avoid this and similar types of bugs.


However, this fix is redundant as the function can be rewritten to avoid a loop altogether. I've also added a few other enhancements:

Function IsInArray(stringToBeFound As String, arr As Variant) As Boolean
  Dim bDimen As Long
  Dim i As Long

  On Error Resume Next
    bDimen = 2
    If IsError(UBound(arr, 2)) Then bDimen = bDimen - 1
    If IsError(UBound(arr, 1)) Then bDimen = bDimen - 1
  On Error GoTo 0

  Select Case bDimen
    Case 0:
    ' Uninitialized array - return false
    Case 1:
      On Error Resume Next
        IsInArray = Application.Match(stringToBeFound, arr, 0)
      On Error GoTo 0
    Case 2:
      On Error Resume Next
        IsInArray = Application.Match(stringToBeFound, Application.Index(arr, 2), 0)
      On Error GoTo 0
    Case Else
      ' Err.Raise vbObjectError + 666, Description:="Never gets here error."
  End Select
End Function

Here is my take on a dictionary solution:

Public Function ProductNumberDict _
                ( _
                           ByVal TheWorksheet As Worksheet, _
                           ByVal Header As String, _
                           ByVal AsGroup As Boolean, _
                           ByVal Start As Long _
                ) _
        As Scripting.Dictionary

  Set ProductNumberDict = New Scripting.Dictionary
  With TheWorksheet.Rows(1).Cells(WorksheetFunction.Match(Header, TheWorksheet.Rows(1), 0)).EntireColumn
    Dim rngData As Range
    Set rngData = TheWorksheet.Range(.Cells(Start), .Cells(Rows.Count).End(xlUp))
  End With
  Dim rngCell As Range
  For Each rngCell In rngData
    With rngCell
      If Not ProductNumberDict.Exists(.Value2) Then
        ProductNumberDict.Add .Value2, IIf(AsGroup, .Offset(, -1).Value2, vbNullString)
      End If
    End With
  Next rngCell
End Function

And here is how to call the function:

Sub UpdatePSI()

  Dim wkstForecast As Worksheet
  Set wkstForecast = ActiveWorkbook.Worksheets("Forecast")

' ...

  Dim dictProductNumbers As Scripting.Dictionary
  Set dictProductNumbers = ProductNumberDict(wkstForecast, "Item", False, 7)
  Set dictProductNumbers = ProductNumberDict(wkstForecast, "Item", True, 3)

  Dim iRowStart As Long: iRowStart = 2
  Dim iFirstCol As Long: iFirstCol = 5
  With wkstForecast.Cells(iRowStart, iFirstCol).Resize(RowSize:=dictProductNumbers.Count)
  .Offset(ColumnOffset:=1).Value = WorksheetFunction.Transpose(dictProductNumbers.Keys)
  .Offset(ColumnOffset:=2).Value = WorksheetFunction.Transpose(dictProductNumbers.Items)
  End With

' ...

End Sub

Note in particular the non-loop method used to copy the dictionary contents to the worksheet.

Sign up to request clarification or add additional context in comments.

1 Comment

I'm choosing this answer because it solved the question I was asking, instead of creating a new way to answer the problem (like I did with my Scripting.Dictionary). Adding the LBound code fixed the problem. Thanks!
1

Per the recommendations of @RonRosenfield and @braX, I tried a Scripting.Dictionary and came up with this answer. It both creates and checks the values, unlike my previous method which used a sub to create and a function to check.

Sub ProductNumberDictionary(strWrkShtName As String, strFindCol As String, blAsGrp As Boolean, iStart As Integer)
    Dim i As Integer
    Dim iLastRow As Integer
    Dim iFindCol As Integer
    Dim strCheck As String

    Set dictProductNumber = CreateObject("Scripting.Dictionary")

    With wbCurrent.Worksheets(strWrkShtName)
        iFindCol = .UsedRange.Find(strFindCol, .Cells(1, 1), xlValues, xlWhole, xlByColumns).Column
        iLastRow = .Cells(Rows.Count, iFindCol).End(xlUp).row
        For i = iStart To iLastRow
            strCheck = .Cells(i, iFindCol).Value
            If dictProductNumber.exists(strCheck) = False Then
                If blAsGrp = False Then
                    dictProductNumber.Add Key:=strCheck
                Else
                    dictProductNumber.Add Key:=strCheck, Item:=.Cells(i, iFindCol - 1).Value
                End If
            End If
        Next
    End With
End Sub

I had some difficulty with getting values from this dictionary, but found that this worked:

    Dim o as Variant
    i = 0
    For Each o In dictProductNumber.Keys
        .Cells(iRowStart + i, iFirstCol + 1) = o 'returns the value of the key
        .Cells(iRowStart + i, iFirstCol + 2) = dictProductNumber(o) 'returns the item stored with the key
        i = i + 1
    Next

1 Comment

Nice one! See my answer for the neat way to extract values from the dictionary.
0

Problems

You are checking for strings inside a variant array. The data could be string or number, therefore giving you duplicates. I suggest changing your function Function IsInArray(stringToBeFound As String, arr As Variant) As Boolean to Function IsInArray(stringToBeFound As Variant, arr() As Variant) As Boolean

There are a few variables that need to be Declared. See below.

Sub ProductNumberArray(strWrkShtName As String, strFindColumn As String, blAsGrp As Boolean, iStart As Integer)
Dim i As long, j as long 'just use long for i.  integers are silently converted to long anyway.  leaving j undeclared makes it variant.
Dim lastRow As Integer
Dim iFindColumn As Integer
Dim checkString As Variant ' changed to variant
Dim arrProductNumber() as Variant ' delcare a dynamic array

ReDim arrProductNumber(0 To 0) ' making it an array

j = 0 'giving somewhere to start

With wbCurrent.Worksheets(strWrkShtName)
    iFindColumn = .UsedRange.Find(strFindColumn, .Range("A1"), xlValues, xlWhole, xlByColumns).Column
    lastRow = .Cells(Rows.Count, iFindColumn).End(xlUp).row
    For i = iStart To lastRow
        checkString = .Cells(i, iFindColumn).Value
        If IsInArray(checkString, arrProductNumber) = False Then
            If blAsGrp = False Then
                ReDim Preserve arrProductNumber(0 To j)
                arrProductNumber(j) = checkString
                j = j + 1
            Else
                ReDim Preserve arrProductNumber(1, 0 To j)
                arrProductNumber(0, j) = .Cells(i, iFindColumn - 1).Value
                arrProductNumber(1, j) = checkString
                j = j + 1
            End If
        End If
    Next i
End With
End Sub

Comments

0

I'm guessing that you are getting duplicates because j and arrProductNumber is are Global variables. You should get rid of the Globals by passing the Worksheet to a function that will return your array.

You could simply add the Cell references to a Scripting.Dictionary

If not dic.Exists(Cell.Value) then dic.Add Cell.Value, Cell

and later retrieve the reference by it's key value

ProductOffset = dic("PID798YD").Offset(0,-1)

Here I use an ArrayList (I could have used a Scripting.Dictionary) to check for duplicates and act as a counter to Redim a multidimensional array.


Sub TestgetProductData()
    Dim results As Variant
    results = getProductData(ActiveSheet, "Column 5", True, 3)
    Stop
    results = getProductData(ActiveSheet, "Column 5", False, 3)
    Stop
End Sub

Function getProductData(ws As Worksheet, ColumnHeader As String, blAsGrp As Boolean, iStart As Integer) As Variant
    Dim results As Variant
    Dim cell As Range, Source As Range
    Dim list As Object
    Set list = CreateObject("System.Collections.ArrayList")

    With ws.UsedRange
        Set Source = .Find(ColumnHeader, .Range("A1"), xlValues, xlWhole, xlByColumns)
        If Not Source Is Nothing Then
            Set Source = Intersect(.Cells, Source.EntireColumn)
            Set Source = Intersect(.Cells, Source.Offset(iStart))
            For Each cell In Source
                If Not list.Contains(cell.Value) Then

                    If blAsGrp Then
                        If list.Count = 0 Then ReDim results(0 To 1, 0 To 0)

                        ReDim Preserve results(0 To 1, 0 To list.Count)
                        results(0, list.Count) = cell.Offset.Value
                        results(1, list.Count) = cell.Value
                    End If
                    list.Add cell.Value
                End If
            Next
        End If
    End With
    If blAsGrp Then
        getProductData = results
    Else
        getProductData = list.ToArray
    End If
End Function

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.