Web scraping using proxy in vba

Question

I've written a script in vba to parse movie names and year from a torrent site. The script is doing just awesome. Although the scraper is leaving no room for complaint, I'm still dubious about how the way I've set the proxy is accurate. Moreover, I've set two proxies in my scraper. Btw, I went through few codes where the proxy has been set using .setProxy 2,"xxxxxxx" whereas I used .setProxy 1 in my below code because the earlier one was throwing errors. Once again, the code is working flawlessly. Thanks in advance.

Here is what I've written:

Sub Torrent_Scraper()
    Dim http As New ServerXMLHTTP60, html As New HTMLDocument
    Dim post As HTMLHtmlElement

    With http
        .Open "GET", "https://yts.ag/browse-movies", False
        .setRequestHeader "Content-Type", "text/html; charset=utf-8"
        .setRequestHeader "User-Agent", "Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36"
        .setProxy 1, "61.233.25.166:80", "46.101.27.218:8118"
        .send
        html.body.innerHTML = .responseText
    End With

    For Each post In html.getElementsByClassName("browse-movie-bottom")
        With post.getElementsByClassName("browse-movie-title")
            x = x + 1: Cells(x, 1) = .item(0).innerText
        End With
        With post.getElementsByClassName("browse-movie-year")
            If .Length Then Cells(x, 2) = .item(0).innerText
        End With
    Next post
End Sub

Answer 1

Here are my suggestions:

Use Application.ScreenUpdating = False to speed up the execution of the code. See here for more details.

Declare elements where you need them. While your code is short (which is great, btw), it's a good habit to get into.

Use Option Explicit and declare that x variable appropriately. x right now is of type Variant but you're only using it like a Long, so you should declare it as Long. Also, change x to a more meaningful name like row.

Don't put two executable statements in the same line.

Having x = x + 1: Cells... is clever, but it may confuse other people later on (or even yourself if you don't touch this code for months).

Set a reference to the active sheet if that's the sheet you intend to use. When you use Cells(...) you are referring to the active sheet. However, while you are looping through your items in the For loop, if you were to click on another sheet, then your data will get messed up when you could avoid this issue pretty easily. In the code below, Cells(...) becomes activeWs.Cells(...).

Personally, I would only use With ... End With if I had many statements I needed it for (like the first use you have of it). In the second and third uses of With, I would just remove it. I would also use If ... Then ... End If so the If statement isn't so long.

I can tell that you wanted to make the code as short as possible, but sacrificing a few lines for clarity will be more beneficial in the long run, in my opinion.

Lastly, about If post.getElementsByClassName("browse-movie-year").Length Then, I would explicitly state what you really mean, which is that if the length if greater than 0. While this statement returns an array and I get that you're checking for an array length and it will always be >=0, it's best to be explicit for clarity.

EDIT: Thinking about this further, this code could be considered to do too much. If you were to follow the Single Responsibility Principle, then we could split out the work to other, private functions.

Here is my version of the refactored code (untested):

Option Explicit

Sub Torrent_Scraper()
    Application.ScreenUpdating = False

    Dim url as String
    Dim proxyServer As Variant
    Dim bypassList as Variant

    url = "https://yts.ag/browse-movies"
    proxyServer = "61.233.25.166:80"
    bypassList = "46.101.27.218:8118"

    Dim html As New HTMLDocument
    html.body.innerHTML = GetHttpResponseTextByProxy(url, proxyServer, bypassList)

    Call SetMovieData(html, ActiveSheet)

    Application.ScreenUpdating = True
End Sub

Private Function GetHttpResponseTextByProxy( _
    ByRef url as String, _
    ByRef proxyServer as Variant, _
    ByRef bypassList as Variant) As String

    Dim http As New ServerXMLHTTP60

    With http
        .Open "GET", url, False
        .setRequestHeader "Content-Type", "text/html; charset=utf-8"
        .setRequestHeader "User-Agent", "Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36"
        .setProxy 1, proxyServer, bypassList
        .send
    End With

    GetHttpResponseTextByProxy = http.responseText
End Function

Private Sub SetMovieData(ByRef html as HTMLDocument, ByRef ws As Worksheet)
    Dim post As HTMLHtmlElement
    Dim row As Long

    For Each post In html.getElementsByClassName("browse-movie-bottom")
        row = row + 1
        ws.Cells(row, 1) = post.getElementsByClassName("browse-movie-title").item(0).innerText

        If post.getElementsByClassName("browse-movie-year").Length > 0 Then
            ws.Cells(row, 2) = post.getElementsByClassName("browse-movie-year").item(0).innerText
        End If
    Next post
End Sub