Pairing all identifiers from 3 sheets and list them on a report worksheet

Question

I have this, and I think it's working? It's been running nearly all day and still isn't finished, so I'm hoping there is a better way to code it.

Right now I have a series of loops that iterate through all my data, but it's taking entirely too long. In short, I have three large worksheets, one master with one identifier, and two others with three possible identifiers.

SSB is the master sheet, containing CUSIP. EDM and BB are the data sheets, which may or may not contain CUSIP, ISIN, and SEDOL.

My goal is to create a new sheet, Identifiers, that compiles all of this data in one sheet. Unfortunately, the only way I know how to do this is loops within loops, these files are 5-30k rows each. I've tried to optimize the code for what it is, but at this point I'm hoping there's just a better method that what I've done. I've been running this all afternoon on a second computer and it's still not done.

As requested, the plain English version of what's going on:

1. Take Column A from SSB as main identifier, CUSIP.
2. Iterate through EDM column I for a match. If found, copy line to report worksheet, Identifiers.
3. If not found, look through BB column A for a match.
4. If a match is found here, check columns D and E, ISIN & SEDOL, against EDM columns G and H. If match is found copy line to Identifiers.
5. If no match of any identifier on any worksheet, copy CUSIP to no identifiers worksheet.
6. Once the list is parsed, check the identifiers worksheet CUSIP, ISIN, and SEDOL against BB and EDM to make sure no identifiers are missing.

Here is what I have:

Sub Identifiers()

Dim SSB As Worksheet
Dim EDM As Worksheet
Dim BB As Worksheet
Dim Identifiers As Worksheet
Dim NoIdentifiers As Worksheet
Dim lastrow As Long
Dim endSSB As Long
Dim endIdentifiers As Long
Dim endNoIdentifiers As Long
Dim CUSIP As Variant
Dim SEDOL As Variant
Dim ISIN As Variant
Dim BBlast As Variant
Dim EDMtable As Variant
Set SSB = Sheets("SSB Holdings")
Set EDM = Sheets("EDM Data")
Set BB = Sheets("BB Data")
Set Identifiers = Sheets("Identifiers")
Set NoIdentifiers = Sheets("No Identifiers")

'text to values
With EDM.UsedRange
    .Value = .Value
End With

With BB.UsedRange
    .Value = .Value
End With

'Clear prior data
Identifiers.Cells.ClearContents
NoIdentifiers.Cells.ClearContents

'Add headers to sheets
EDM.Range("A1:BP1").Copy
Identifiers.Range("A1").PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
    Application.CutCopyMode = False
SSB.Range("A1:B1").Copy
NoIdentifiers.Range("A1").PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
    Application.CutCopyMode = False



'compile identifiers
i = 0
lastrow = EDM.Range("A" & Rows.Count).End(xlUp).Row
BBlast = BB.Range("A" & BB.Range("A" & Rows.Count).End(xlUp).Row)
EDMtable = EDM.Range("A1:I" & EDM.Range("A" & Rows.Count).End(xlUp).Row)

'Remove lines with no identifiers

For i = 2 To lastrow
    If IsEmpty(EDM.Range("G" & i)) Then
        If IsEmpty(EDM.Range("H" & i)) Then
            If IsEmpty(EDM.Range("I" & i)) Then
                Range("A" & i).EntireRow.Delete
            End If
        End If
    End If
Next i

'Filter SSB CUSIPs
endSSB = SSB.Range("A" & Rows.Count).End(xlUp).Row

For i = 2 To endSSB
    CUSIP = SSB.Range("A" & i)
        For x = 2 To lastrow
            If EDM.Range("I" & x).Value = CUSIP Then
                endIdentifiers = Identifiers.Range("A" & Rows.Count).End(xlUp).Row
                EDM.Range("I" & x).EntireRow.Copy
                Identifiers.Range("A" & endIdentifiers + 1).PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, _
                    SkipBlanks:=False, Transpose:=False
                Application.CutCopyMode = False
            ElseIf EDM.Range("I" & x).Value <> CUSIP Then
                For y = 2 To BB.Range("A" & Rows.Count).End(xlUp).Row
                    If BB.Range("A" & y) = CUSIP Then
                        ISIN = BB.Range("D" & y)
                        SEDOL = BB.Range("E" & y)
                            For p = 2 To lastrow
                                If EDM.Range("G" & p).Value = ISIN Then
                                    endIdentifiers = Identifiers.Range("A" & Rows.Count).End(xlUp).Row
                                    EDM.Range("G" & p).EntireRow.Copy
                                    Identifiers.Range("A" & endIdentifiers + 1).PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, _
                                        SkipBlanks:=False, Transpose:=False
                                    Application.CutCopyMode = False
                                ElseIf EDM.Range("H" & p).Value = SEDOL Then
                                    endIdentifiers = Identifiers.Range("A" & Rows.Count).End(xlUp).Row
                                    EDM.Range("G" & p).EntireRow.Copy
                                    Identifiers.Range("A" & endIdentifiers + 1).PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, _
                                        SkipBlanks:=False, Transpose:=False
                                    Application.CutCopyMode = False
                                End If
                            Next p
                    End If
                Next y
            Else: SSB.Range("A" & i).EntireRow.Copy
                NoIdentifiers.Range("A" & endIdentifiers + 1).PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, _
                    SkipBlanks:=False, Transpose:=False
                Application.CutCopyMode = False
            End If
        Next x
Next i

'Fill in CUSIP/ISIN/SEDOL in identifiers tab
endIdentifiers = Identifiers.Range("A" & Rows.Count).End(xlUp).Row
For i = 2 To endIdentifiers
    If IsEmpty(Identifiers.Range("A" & i)) Then
            ISIN = Identifiers.Range("G" & i)
            SEDOL = Identifiers.Range("H" & i)
            For x = 2 To BBlast
                If BB.Range("D" & x) = ISIN Then
                    Identifiers.Range("A" & i).Value = BB.Range("A" & x)
                    If IsEmpty(Identifiers.Range("A" & i)) Then
                        For p = 2 To BBlast
                            If BB.Range("E" & p) = SEDOL Then
                                Identifiers.Range("A" & i).Value = BB.Range("A" & p)
                            End If
                        Next p
                    End If
                End If
            Next x
    End If
Next i

endIdentifiers = Identifiers.Range("A" & Rows.Count).End(xlUp).Row
For i = 2 To endIdentifiers
    If IsEmpty(Identifiers.Range("G" & i)) Then
            CUSIP = Identifiers.Range("A" & i)
            SEDOL = Identifiers.Range("H" & i)
            For x = 2 To BBlast
                If BB.Range("A" & x) = CUSIP Then
                    Identifiers.Range("G" & i).Value = BB.Range("D" & x)
                    If IsEmpty(Identifiers.Range("G" & i)) Then
                        For p = 2 To BBlast
                            If BB.Range("E" & p) = SEDOL Then
                                Identifiers.Range("G" & i) = BB.Range("D" & p)
                            End If
                        Next p
                    End If
                End If
            Next x
    End If
Next i

endIdentifiers = Identifiers.Range("A" & Rows.Count).End(xlUp).Row
For i = 2 To endIdentifiers
    If IsEmpty(Identifiers.Range("H" & i)) Then
            CUSIP = Identifiers.Range("A" & i)
            ISIN = Identifiers.Range("G" & i)
            For x = 2 To BBlast
                If BB.Range("A" & x) = CUSIP Then
                    Identifiers.Range("H" & i).Value = BB.Range("E" & x)
                    If IsEmpty(Identifiers.Range("H" & i)) Then
                        For p = 2 To BBlast
                            If BB.Range("D" & p) = ISIN Then
                                Identifiers.Range("H" & i) = BB.Range("E" & p)
                            End If
                        Next p
                    End If
                End If
            Next x
    End If
Next i


End Sub

Answer 1

Let's start with your actual question, i.e. with reducing the loops.

If you look closely, you can see that in 'Filter SSB CUSIPs' your second inner loop is independent of the surrounding loop. More precisely, nothing in the loop with parameter y depends on x. Accordingly, you can execute the loop for y after that for x. This would also result in the intended search order. Similarly, you can reduce loops in the other parts by extracting independent inner loops.

Actually, you can get rid of many explicite loops by using the Range.Find method. This returns the first match for a value in the range it is invoked on. If there is no match, it returns Nothing.

A further way to improve the speed of the code would be not to use Copy and PasteSpecial but to read the values in the rows into a two dimensional array (best using Range.Value2) and write them out from that array again. (See this blog post.)

Dim rowArray As Variant
rowArray = Sheet1.Range("A1:A16").Value2
Sheet2.Range("B12").Value = rowArray 

Finally, you could keep track of the row into which you pasted last. Then you could avoid querying Excel for the last row each and every time you copy rows.

Now, I would like to add some general stylistic comments:

1. It is rather hard to understand what your code is doing because you heavily rely on default members, in particular on the default member of a Range, namely Value. It is not always clear what you are actually comparing.
2. Your code could benefit from splitting it into logical pieces. Basically, everywhere where you have a caption comment you could extract the code following the caption into its own aptly named Sub and just call it. Another example is that you could extract a sub that handles copying the rows. This would remove quite some code duplication.
3. In 'Remove lines with no identifiers' you can avoid the nested if blocks by using And to combine the conditions.
4. It is a good practice to use longer more expressive names for variables in favor of short unexpressive ones. This makes the code much easier to read for people without intimate knowledge of the code.

Looking at your code again, I see that it is actually not doing what you intend to do. The problem is that in 'Filter SSB CUSIPs' in the elseif the condition is just the opposite of that in the if statement. Thus, the else branch is dead code.

Finally, if your tables are very big, you might consider to do a merge join instead of the two outer loops in 'Filter SSB CUSIPs'. More precisely, it might be good to load the first column of SSB, and the tables EDM and BB into arrays, sort these by identifier using your favorite n*log(n) sorting algorithm, and then step through them in order.

Answer 2

@M.Doerner addresses some possible optimizations to the loop structure, but I think it's important to illustrate why this code is (probably) still running. Let's count the iterations. For the sake of discussion, let's assume that each of the 3 sheets has the minimum number of rows from your estimate, 5,000 and do the math.

Your first loop is this:

For i = 2 To 5000
    For x = 2 To 5000
        'Optionally
        For y = 2 To 5000
            For p = 2 To 5000
            Next p
        Next y
    Next x
Next I

That's 5000 * 5000 iterations for the outer 2 loops and 5000 * 5000 for the inner 2 loops. Lets be really conservative and say that the optional inner loops run for only 10% of all cases due to your If condition. That's (25m) * (2.5m), or 62,500,000,000,000 iterations. Now let's be really optimistic and say that Excel can do a million of those per second (it can't). Using those assumptions, your first nested loop structure should finish sometime in early 2019. If each of the 3 sheets has 30k records, we're looking at somewhere in the neighborhood of 2500 years for it to complete.

The next 3 nested looping structures are similar, but hardly relevant as by the time you reach that code the chances of an equipment failure, forced restart, etc. will be trending toward 100%.

---

So, since we don't want to wait that long, lets step back and break the task down into smaller pieces:

1. Take Column A from SSB as main identifier, CUSIP.

2. Iterate through EDM column I for a match. If found, copy line to report worksheet, Identifiers.

These are not conditional, so pull this step out and do it by itself. A fairly efficient way to do this is to build a lookup of "CUSIP" to row number from the EDM sheet with a Scripting.Dictionary, and then go through the SSB "CUSIP" column and see if they're in the lookup. While you're doing this, keep another hashset of "CUSIP"s that are unmatched, and then use them on the next pass.

3. If not found, look through BB column A for a match.

4. If a match is found here, check columns D and E, ISIN & SEDOL, against EDM columns G and H. If match is found copy line to Identifiers.

OK, so at this point we have a hashset of "CUSIP" that are unmatched. That means we only need to test ones that didn't match. We also have a lookup of "CUSIP" to rows in the EDM sheet that we built on the first pass. So, go through the BB sheet and see if they're in the "unmatched" hashset. If they are, look up the row in your EDM lookup and test the other values.

The rest is just moving data around.

---

OK, that's probably doing much better now. But what if there were a way to link multiple tables of data using common identifiers? Well, you're in luck - that sounds a lot like SQL, which is supported by Excel via ADODB. The first step boils down to this:

SELECT {field list} FROM [SSB Holdings] S RIGHT JOIN [EDM Data] E ON S.CUSIP = E.CUSIP

The second is probably something like this (although I'm not 100% clear on your critera):

SELECT {field list} FROM [SSB Holdings] S RIGHT JOIN [BB Data] B ON S.CUSIP = B.CUSIP
RIGHT JOIN [EDM Data] E ON E.ISIN = B.ISIN AND E.SEDOL = B.SEDOL

You finally pick up the unmatched ones with something like:

SELECT {field list} FROM [SSB Holdings] S LEFT JOIN [Identifiers] I ON S.CUSIP = I.CUSIP
WHERE I.CUSIP IS NULL

...or...

SELECT {field list} FROM [SSB Holdings] WHERE CUSIP NOT IN (SELECT CUSIP FROM [Identifiers])

---

While you probably could come up with an algorithm in VBA that does this fairly efficiently, I would doubt you'll match a relational database engine in performance on this. The take-away here is that if you're looking for the best way to skin a cat, make sure the cat needs to be skinned first.