4
\$\begingroup\$

I have 7 source files that I am searching and cross-referencing. I have tried a few different methods inside Excel (cell-by-cell iteration, dictionary), and found select statements as recordsets to be the fastest thus far.

Unfortunately, it still takes around 5 seconds for my code to search 4 of the files for all instances of a string from another file. Since this file currently has 9,000 (and will soon grow to over half a million), I'm wondering how I can improve the speed.

The 7 files are structured as follows:

  • String-Source(10+ fields, only 2 are needed, current product listing downloaded in .xlsx format from a website)
  • 4 vendor files (.csv or tab delimited or .xls or the like, with/without/maybe-sometimes headers, ranging in size from small [only 5,000 records] to larger [about a million] records, 20+ fields but only 3 are needed)
  • sheets("yes") and sheets("no") (3 fields each, very small less than 1000, only used when I find a match, of which I only expect less than 100,000 hits each, so I am focusing less on optimizing search here)

The purpose of my code:

I have a key value that will be identical across the source and vendor files. Unfortunately, this value might be found multiple times in my source file, and multiple times or not at all in my vendor files. This value represents a model number, unique to the manufacturer, but each vendor includes multiple manufacturers in their files.

A problem is that the manufacturer name may (often does) have a different spelling, not only from file to file, but also within vendor files. Therefore I can't just concatenate the two fields into one and search for matches. What I have been doing is finding every instance of the model number, and comparing the manufacturer name fields to see if they refer to the same company (sometimes the manufacturer is spelled out, sometimes acronym, sometimes location appended, sometimes sub-brand used instead of or in addition to manufacturer, and often just spelled wrong. Oh, and many of the manufacturers in this industry have similar names.)

This comparison is being done by hand by a human and go into my yes/no sheets, so the human only has to answer the same question one time, regardless of the number of times this report is run. So, every time the model number is identical, I search the yes/no sheets, and if 'yes' then they are the same product and the 'ID' (third field, unique by file) is output for use by a different process.

Sub matchprod2()
   Application.ScreenUpdating = False
   Dim FoundCell As Range
   Dim LastCell As Range
   Dim FirstAddr As String
   Set myworkbook = ThisWorkbook    
   Dim cn As Object: Dim thisbook As Object
   Set cn = CreateObject("adodb.connection"): Set thisbook = CreateObject("adodb.connection")
   strfilename = ThisWorkbook.Path
   cn.connectionstring = _
           "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & strfilename & _
           ";Extended Properties=""text;HDR=NO;imex=1"";"
   cn.Open
   thisbook.connectionstring = _
           "Provider=Microsoft.Jet.OLEDB.4.0;Data Source=" & ThisWorkbook.FullName & _
           ";Extended Properties=""Excel 8.0;HDR=Yes;IMEX=1"";"
   thisbook.Open
   Set thisrs = thisbook.Execute("select * from [SourceProducts$a1:c9111]")
   a = 1
   Do Until thisrs.EOF
        a = a + 1
        mfg = thisrs(0)
        modelnum = thisrs(2)
        myworkbook.Sheets("matches").Cells(a, 1).Value = mfg
        myworkbook.Sheets("matches").Cells(a, 2).Value = modelnum

        Set rs1 = cn.Execute("select Field6, Field1 from invfeed.csv where Field2='" & modelnum & "'")
        Set rs2 = <same as rs1, different file>
        Set rs3 = <same as rs1, different file>
        Set rs4 = <same as rs1, different file>
        Do Until rs1.EOF
            If Trim(rs1("Field6")) = Trim(mfg) Then
                If myworkbook.Sheets("matches").Cells(a, 2 + 1) <> "" Then myworkbook.Sheets("matches").Cells(a, 2 + 1).Value = "multiple": Exit Do
                myworkbook.Sheets("matches").Cells(a, 2 + 1) = rs1("Field1")
            Else
                If comparenames2(Trim(mfg), Trim(rs1("Field6")), "Vendor1", thisbook) Then myworkbook.Sheets("matches").Cells(a, 2 + 1) = "'" & rs1("Field1")
            End If
            rs1.movenext
        Loop
        Set rs1 = Nothing
        Do Until rs2.EOF
<same as rs1, different file>
            rs2.movenext
        Loop
        Set rs2 = Nothing
        Do Until rs3.EOF
<same as rs1, different file>
            rs3.movenext
        Loop
        Set rs3 = Nothing
        Do Until rs4.EOF
<same as rs1, different file>
            rs4.movenext
        Loop
        Set rs4 = Nothing
        thisrs.movenext
    If a Mod 50 = 0 Then Debug.Print a; Now()
    Loop

    Set thisrs = Nothing
    cn.Close
    thisbook.Close
    Set cn = Nothing
    Set thisbook = Nothing
    Application.ScreenUpdating = True
End Sub

Public Function comparenames2(Sourcename, Manname, invfilesource, wkbkconn) As Boolean
    If IsNull(Manname) Then comparenames2 = False: Exit Function
    Set yeslist = wkbkconn.Execute("select [Source Manufacturer], [Vendor Manufacturer] from [yes$A:B] where [Source Manufacturer]='" & Sourcename & "'")
    Do Until yeslist.EOF
        If yeslist(1) = Manname Then comparenames2 = True: Exit Function
        yeslist.movenext
    Loop
    Set nolist = wkbkconn.Execute("select [Source Manufacturer], [Vendor Manufacturer] from [no$A:B] where [Source Manufacturer]='" & Sourcename & "'")
    Do Until nolist.EOF
        If nolist(1) = Manname Then comparenames2 = False: Exit Function
        nolist.movenext
    Loop

    Application.ScreenUpdating = True
    result = MsgBox("Are the following Manufacturers the same?" & vbNewLine & Sourcename & vbNewLine & Manname, vbYesNo + vbQuestion + vbSystemModal)
    Application.ScreenUpdating = False
    If result = vbYes Then mysheetname = "yes" Else mysheetname = "no"
    mylastrow = ThisWorkbook.Sheets(mysheetname).Cells.Find(what:="*", after:=ActiveSheet.Cells(1, 1), SearchOrder:=xlByRows, SearchDirection:=xlPrevious).Row
    ThisWorkbook.Sheets(mysheetname).Cells(mylastrow + 1, 1) = Sourcename
    ThisWorkbook.Sheets(mysheetname).Cells(mylastrow + 1, 2) = Manname
    ThisWorkbook.Sheets(mysheetname).Cells(mylastrow + 1, 3) = invfilesource
    If result = vbYes Then comparenames2 = True Else comparenames2 = False
End Function
\$\endgroup\$

migrated from stackoverflow.com May 19 '16 at 14:59

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ microsoft.com/en-us/download/details.aspx?id=39379 Try to see if power query (free excel addin by microsoft) will meet your data analysis needs. You need to focus on not pulling the entire "DB" in statements like the following: "select * from [SourceProducts$a1:c9111]" by adding in "WHERE" statements. Hopefully powerquery makes this a little easier on you. \$\endgroup\$ – Cody G. Apr 28 '16 at 15:53
  • 2
    \$\begingroup\$ Ignore my previous comment, which part of your code is taking the majority of the time? \$\endgroup\$ – Cody G. Apr 28 '16 at 16:01
  • \$\begingroup\$ Wow. I am surprised that an Add-In could speed things up. I have always had the opposite experience. I would need to purchase another operating system since I am working with Windows10 right now, but that is not necessarily an issue. I do have access to remote desktops with Server 2008 that the client rents, so it can be set up there after I build it. I will go ahead and take a look at how an add in could be faster than SQL. Thanks for the tip. In the meantime, I would like to learn how to do it myself instead of depending on an Add-in that doesn't seem to support current Microsoft releases. \$\endgroup\$ – CWilson Apr 28 '16 at 16:15
  • 2
    \$\begingroup\$ More technical, but there are many "free" database technologies that are great for this, especially if you are looking to scale. No offense, but if your growing into the "half a million" records phase, I think your going to need to hire an engineer, and build something a bit more robust than an excel macro. Excel's a great output format, but it is not an application framework. Even MySQL would be better than excel for this. \$\endgroup\$ – Mitchell Lee May 19 '16 at 15:41
  • 1
    \$\begingroup\$ Not to say I wouldn't appreciate suggestions from experienced programmers. Or that I am not interested in fostering a relationship with this site as well. If anyone here wants to give suggestions to this problem, that would be great... but the answer I already came up with was a simple two word answer - "left join". Not impressive enough to answer and mark my own question. A little embarassing I hadn't thought of it before posting. Perhaps I should have deleted this question on SO sooner? I didn't think it was really that urgent, and figured that maybe there was an even better one? I'm no DBA \$\endgroup\$ – CWilson May 20 '16 at 20:32
3
\$\begingroup\$

The moment you start referring to file contents using X Million (even if X is a fraction) then you're well beyond the point where Excel is an appropriate application framework.

For robustness and future scalability, and given your client's inclination to the Microsoft Environment, I would recommend your client gets a license for Microsoft SQL Server (The express version is free and will handle up to 10GB of data) and builds a framework using that.


For your code as it stands, you might want to try Application.EnableEvents = False and Application.Calculation = xlManual, though since you're mainly working with ADODB and recordsets, it might not make much of a difference.

\$\endgroup\$
  • \$\begingroup\$ Thank you. I am no programmer, and I have only worked with MS SQLServer twice (at a Fortune50 finance company, and a manufacturing firm), and getting it set up is daunting. Of course housing all the data there would be best, but I am a little chicken to start using it. I would even be more comfortable setting this up in Base SAS, but I can't exactly justify a recommendation of spending that kind of money here. You are of course, correct, though. \$\endgroup\$ – CWilson May 20 '16 at 20:41
  • \$\begingroup\$ Oh, and EnableEvents=False kind of messed with the msgbox in my function. But that whole function really wasn't my best idea ever anyway: worked much better to just output the matches into a table/spreadsheet/whatever and mark them in order, than sitting in front of the screen like a fool and waiting for the magic msgbox to pop up. \$\endgroup\$ – CWilson May 20 '16 at 20:42
  • \$\begingroup\$ I wouldn't discount a server on the grounds of being too daunting. I'm in exactly the same position right now (a year of VBA development, looking to move my company's records into a server) so I'm getting myself set up with MS SQL Server. So far, I'm about 13 hours into it and nearly at the point where I can start trying to build a system with it. \$\endgroup\$ – Kaz May 22 '16 at 10:19
  • \$\begingroup\$ I agree and disagree, certain data types, modern hardware like SSDs, and good code can make short work of millions of records in excel. If slow to no growth is expected, an already proven excel procedure could continue to be of use. \$\endgroup\$ – emican Jun 15 '16 at 1:43
3
\$\begingroup\$

If you really want to do it in excel, make sure to use at least the tools available to the fullest.

Let's go through the design choices one by one now, to see where things went south:

  • Attempting to query across 3 different files. Working with multiple files is usually a bad design choice. Load your data into tables of the same excel sheet, so you can make proper use of SQL queries.
  • Using two different tables for accepted and denied pairs of vendor names. May not look bad at first, but you should really have used only one table, and a 3rd column denoting "yes" or "no" explicitly. You should probably be changing this now.
  • Using SQL, but not making use of relational algebra. SQL is a powerful tool, if you learn how to master it. Some may say that Excel isn't suited for such tasks, but the reality is that the choice of tools is less relevant than how you use them.

Well, let's now assume that you had fixed the table layout of the valid/invalid pair sheet, and that you already had loaded all files into different tables of a single sheet, and that all columns you need are named. We also assumes that your data is already properly conditioned, that means trim() and alike have already been applied.

Now your task is as simple as writing a single query to do your complete job:

SELECT * FROM ([SourceProducts$]
INNER JOIN [VendorProduct$] ON [SourceProducts$].[ProductID] = [VendorProducts$].[ProductID])
LEFT JOIN [SameVendor$] ON (
    [SourceProducts$].[Manufacturer] = [SameVendor$].[Source Manufacturer]
    AND [VendorProducts$].[Manufacturer] = [SameVendor$].[Vendor Manufacturer]
)
WHERE [SameVendor$].[Is same] IS NULL OR [SameVendor$].[Is same] = 'Yes'

That gives you a complete list of all matched products, excluding those which are already known to be false-positives, but including those who are still unknown. The latter ones can be identified by having empty fields for the columns pulled in from the SameVendor table.

Why we needed a single sheet? Because that's the prerequisite for using JOIN. What you did manually is not much different from doing a JOIN, except that you essentially constructed the temporary table required for that by hand.

\$\endgroup\$
  • \$\begingroup\$ Thank you. No, I didn't really want to do this in Excel. Someone helpful edited that out when they re-wrote the question, because that part was relevant only before it was migrated to CR. I have been doing it in Access for a few weeks, and so I like the table suggestion, but I didn't know that such tables existed in Excel. I will certainly look that up some time. I can't imagine that expand-ability beyond the simple initial 4 'vendors' would be easy using tables embedded in an Excel sheet, but perhaps this is a tip I can use for once off projects in the future. \$\endgroup\$ – CWilson May 23 '16 at 8:12
  • \$\begingroup\$ The SQL statement, though, is very applicable to what I am still doing, and actually looks like it will improve performance over the left join that I have been doing. I like the idea of joining all sources at once, instead of joining then comparing. This is exciting to me, because it reminds me to think beyond just the a/b join that was in my head, limiting my creativity. \$\endgroup\$ – CWilson May 23 '16 at 8:17
  • \$\begingroup\$ Oh, and the 'properly conditioned' comment made me laugh, thank you. Yes, that is the majority of our code now. I don't understand why these files are being curated by hand anyway, but my client is buying them from someone else and we don't have say in that. Why to use apostrophes at all, let alone allow typos like multiple apostrophes strung together or in different places, just as an example, or change your data from integer or decimal (531) (8.2) to string ('0000531') ('0008.20') without warning, and back again... That is what keeps me up at night. \$\endgroup\$ – CWilson May 23 '16 at 8:42
2
\$\begingroup\$
 Public Function comparenames2(Sourcename, Manname, invfilesource, wkbkconn) As Boolean

All your arguments are being passed ByRef which is usually not needed. It looks like you can pass most of those ByVal which may decrease resources. You should also make your function Private and give it a better name.

 Private Function CanCompare(ByVal sourceName as String, manName as String, invFileSource as String, ByRef workbookConnection as Workbook) As Boolean

You'll see I gave the arguments types. When you don't define your variable, VBA will declare it as a Variant, which are objects:

Performance. A variable you declare with the Object type is flexible enough to contain a reference to any object. However, when you invoke a method or property on such a variable, you always incur late binding (at run time). To force early binding (at compile time) and better performance, declare the variable with a specific class name, or cast it to the specific data type.

By not declaring variables, you could possibly be paying a penalty.

I also followed Standard VBA naming conventions, which has camelCase for local variables and PascalCase for other variables and names.

\$\endgroup\$
  • \$\begingroup\$ This is a good answer, but it's relatively common to pass args ByRef for performance reasons when it would otherwise be best practice to pass them ByVal. This is particularly common when passing a large number of large strings around. It's a micro-optimization at best, but there can be a performance penalty to passing ByVal. \$\endgroup\$ – RubberDuck May 22 '16 at 11:48
  • \$\begingroup\$ I want to learn more about this naming suggestion. I had always been taught that naming makes no difference at either compile or run time for VBA, whether for variables, functions, or any user created type. Why should the function be given "a better name?" By the way, the link tells me the conventions Microsoft chose for documenting online help files for the VB language, not naming conventions for VBA. \$\endgroup\$ – CWilson May 23 '16 at 7:58
  • \$\begingroup\$ I had no idea that not declaring variable types inside the arguments would pass them as variants. Thank you for that. \$\endgroup\$ – CWilson May 23 '16 at 7:58
  • \$\begingroup\$ Better naming makes it easier to read. \$\endgroup\$ – Raystafarian May 23 '16 at 8:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.