Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    text
    copied!<p>There are a number of small issues and a few larger opportunities here. You did say this is your first vba work, so forgive me if I'm telling you things you already know </p> <p>Small things first:<br> <code>Dim file, a, b, c As Integer</code> declares file, a and b as variants. <code>Integer</code> is 16 bit sign, so there may be risk of overflows, use <code>Long</code> instead.</p> <p><code>DIM</code>'ing inside loops is counter-productive: unlike C++ they are not loop scoped.</p> <p>The real opportunity is: </p> <p>Use <code>For Each</code> where you can to iterate collections: its faster than indexing.</p> <p>On my hardware your original code ran in about 160s. This code in about 2.5s (both plus time to load word file into the collection, about 4s)</p> <pre><code>Sub build_trie() Dim t1 As Long Dim wd As Variant Dim nd As Node Set tree = New Node ' Dim file, a, b, c As Integer : declares file, a, b as variant Dim file As Integer, a As Long, b As Long, c As Long ' Integer is 16 bit signed Dim current As Node Dim wordlist As Collection Set wordlist = New Collection file = FreeFile Open "C:\corncob_caps.txt" For Input As file ' no point in doing inside loop, they are not scoped to the loop Dim line As String Dim match As Boolean Dim char As String Dim new_node As Node Do While Not EOF(file) 'Dim line As String Line Input #file, line wordlist.Add line Loop t1 = GetTickCount For Each wd In wordlist ' for each is faster 'For a = 1 To wordlist.Count Set current = tree For b = 1 To Len(wd) 'Dim match As Boolean match = False 'Dim char As String char = Mid$(wd, b, 1) For Each nd In current.next_nodes 'For c = 1 To current.next_nodes.Count If char = nd.letter Then 'If char = current.next_nodes.Item(c).letter Then Set current = nd 'Set current = current.next_nodes.Item(c) match = True Exit For End If Next nd If Not match Then 'Dim new_node As Node Set new_node = New Node new_node.letter = char current.next_nodes.Add new_node Set current = new_node End If Next b current.is_word = True Next wd Debug.Print "Time = " &amp; GetTickCount - t1 &amp; " ms" End Sub </code></pre> <p>EDIT:</p> <p>loading the word list into a dynamic array will reduce load time to sub second. Be aware that Redim Preserve is expensive, so do it in chunks</p> <pre><code> Dim i As Long, sz As Long sz = 10000 Dim wordlist() As String ReDim wordlist(0 To sz) file = FreeFile Open "C:\corncob_caps.txt" For Input As file i = 0 Do While Not EOF(file) 'Dim line As String Line Input #file, line wordlist(i) = line i = i + 1 If i &gt; sz Then sz = sz + 10000 ReDim Preserve wordlist(0 To sz) End If 'wordlist.Add line Loop ReDim Preserve wordlist(0 To i - 1) </code></pre> <p>then loop through it like </p> <pre><code> For i = 0 To UBound(wordlist) wd = wordlist(i) </code></pre>
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload