Code runs too slow, can it be improved

[Sharid]

Hi can somebody please help

Can this code be written better, so it processes the URL faster, it took almost 3 hours to process 1000 URL


What the code does
I have a url list in Sheet2 "URL LIST" which it uses to process and then puts result in Sheet1

[COLOR=#ff0000]'Count url in sheet2[/COLOR]
With Worksheets("URL LIST")
      lastRow = .Cells(.Rows.Count, "A").End(xlUp).Row
           Sheets("URL LIST").Range("L1").Value = lastRow
      End With
  
[COLOR=#ff0000]' Run main code[/COLOR]
 Dim wb As Workbook
 Dim x As Variant
 Dim i, j, k, l As Integer
 Dim r As Long, lr As Long
 Dim wsSheet As Worksheet, links As Variant, ie As Object, link As Variant
 Dim rw As Long


    i = 2
    k = 2
    l = 2
[COLOR=#ff0000]    'SHEET2 as sheet with URL[/COLOR]
    Set wb = ThisWorkbook
    Set wsSheet = wb.Sheets("URL LIST")
    
[COLOR=#ff0000]    'Set IE = InternetExplorer[/COLOR]
    Set ie = CreateObject("InternetExplorer.Application")
    
    rw = wsSheet.Cells(wsSheet.Rows.Count, "A").End(xlUp).Row
    links = wsSheet.Range("A1:A" & rw)
    
[COLOR=#ff0000]    'IE Open Time per page 5sec and check links on Sheet2 Column A[/COLOR]
    With ie
       .Visible = True
       Application.Wait (Now + TimeValue("00:00:5"))
       
       For Each link In links
           .navigate (link)
           While .Busy Or .readyState <> 4: DoEvents: Wend


Dim doc As HTMLDocument [COLOR=#ff0000]'variable for document or data which need to be extracted out of webpage[/COLOR]
Set doc = ie.document
Dim dd As Variant
On Error Resume Next
dd = doc.getElementsByClassName("[COLOR=#0000cd]PUT CLASS HERE[/COLOR]")(0).Children(0).href
On Error Resume Next


[COLOR=#ff0000]'Paste in this sheet[/COLOR]
Sheet1.Cells(Sheet1.Cells(Sheet1.Rows.Count, "A").End(xlUp).Row + 1, "A").Value = dd
         
[COLOR=#ff0000]'Deletes duplicates in column A Sheet1[/COLOR]
    'Columns(1).RemoveDuplicates Columns:=Array(1)
    
[COLOR=#ff0000] ' Put no1 in sheet2 column F[/COLOR]
  Sheets("URL LIST").Range("F" & Rows.Count).End(xlUp).Offset(1, 0).Value = 1
 
[COLOR=#ff0000] 'Count No1 in sheet2 Column F[/COLOR]
With Worksheets("URL LIST")
    lastRow = .Cells(.Rows.Count, "F").End(xlUp).Row
    Sheets("URL LIST").Range("L2").Value = lastRow
    End With


[COLOR=#ff0000]'Click CommandButton9 [/COLOR]
Call CommandButton9_Click


[COLOR=#ff0000]'navigate links[/COLOR]
       Next link
       
[COLOR=#ff0000]'Close IE Browser[/COLOR]
    .Quit
    End With
    
    Set ie = Nothing

Thanks for having a look

 

[jproffer]

Are we trying to gain seconds...milliseconds...minutes...hours? How "slow" is it running now? Looks like you're navigating to (some number) of webpages, with a 5 second delay on each one...and an unknown delay until the readystate = 4.

At any rate, what's your run time right now, approximately?

On another note, from what I understand, this:

Dim i, j, k, l As Integer

isn't the same as this:

Dim i as integer
Dim j as integer
Dim k as integer
Dim l as integer

And on another note, using a lowercase L as a variable is asking for trouble, IMHO. Looks too much like a 1...but to each their own

 

[Sharid]

I wrote some myself, some with forum help as i'm very limited in vba.

It take 3hr or more to do process 1000 urls.


Also along the code runs much faster that it currently does I'm happy.

 

[JackDanIce]

Untested, this may be quicker:

Sub Macro1()

    Dim LR      As Long
    Dim x       As Long
    Dim arr()   As Variant
    
    Dim wks     As Worksheet: Set wks = ThisWorkbook.Sheets("URL LIST")
    Dim ie      As Object: Set ie = CreateObject("InternetExplorer.Application")
    Dim doc     As HTMLDocument: Set doc = ie.document
    Dim dd      As Variant
    
    Application.ScreenUpdating = False
    
    With wks
        LR = .Cells(.Rows.Count, 1).End(xlUp).Row
        .Cells(1, 12).Value = LR
        arr = .Cells(1, 1).Resize(LR).Value
    End With
    
    With ie
        .Visible = True
        Application.Wait Now + TimeValue("0:00:5")
        
        For x = LBound(arr, 1) To UBound(arr, 1)
            .navigate arr(x, 1)
            While .busy Or .readystate <> 4: DoEvents: Wend
            
            On Error Resume Next
            With Sheet1
                LR = .Cells(.Rows.Count, 1).End(xlUp).Offset(1).Row
                .Cells(LR, 1).Value = doc.getElementsByClassName("put class here")(0).Children(0).href
                .Cells(1, 1).Resize(LR).RemoveDuplicates Columns:=Array(1)
            End With
            On Error GoTo 0
            
            wks.Cells(wks.Cells(Rows.Count, 6).End(xlUp).Offset(1).Row).Value = 1
            wks.Cells(2, 12).Value = Application.CountIf(wks.Cells(1, 1).Resize(LR), 1)
             
            Call CommandButton9_Click
        Next x
        .Quit
    End With
    
    Set arr = Nothing
    Set wks = Nothing
    Set ie = Nothing
    Set doc = Nothing
    
End Sub

Though @jproffer's observations still hold, e.g. the 5 second wait

Also, my understanding is

 Dim i, j, k, l As Integer

Declares i, j and k as type Variant with only l as type Integer. In addition, in modern Excel, the complier auto converts type integers to type longs.

You're better declaring them independently (again as suggested byjproffer) since variants use even more memory space than longs (which in turn are bigger than integers but that's a moot comment)

I'd also avoid declaring variables in random places in the code, better to have them all grouped at the top, but that's more programming style than make-or-break.

 

[Sharid]

I get an error here, of the last bit

Set arr = Nothing

Compiler Error - Can't assign to array

 

[JackDanIce]

Change that to:

Erase arr

 

[Sharid]

I am now getting a problem here - this is the error message "Method 'Document' of object 'IWebBrowser2' failed"

 Dim doc     As HTMLDocument: Set doc = [COLOR=#ff0000]ie.document[/COLOR]

 

[JackDanIce]

That's based on your code here:

Dim doc As HTMLDocument 'variable for document or data which need to be extracted out of webpage
Set doc = ie.document

The only difference is I've used ":" (a colon) for line continuation

Is the object HTML library turned on? See following links:

https://stackoverflow.com/questions/45825995/excel-visual-basic-variable-as-htmldocument
http://vbadud.blogspot.com/2009/08/how-to-login-to-website-using-vba.html

 

[Sharid]

yes object HTML library is one, but still getting the error message

 

[JackDanIce]

Have you tried replacing that single line back to:

Dim doc As HTMLDocument 'variable for document or data which need to be extracted out of webpage
Set doc = ie.document

If it works in your existing (but slow) macro, then you're doing a direct like-for-like replacement with code that works for you.



In addition, have you tried searching the internet for: Error "Method 'Document' of object 'IWebBrowser2' failed"

I typed that in, these are the results that came back

The first result returned is here: https://stackoverflow.com/questions/30086425/excel-vba-method-document-of-object-iwebbrowser2-failed which contains an answer that suggests a fix for that specific error