Navigation Menu

Skip to content

Instantly share code, notes, and snippets.

@sancarn
Last active December 26, 2022 05:31
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save sancarn/91384cf831d235b0415d40d582ca951c to your computer and use it in GitHub Desktop.
Save sancarn/91384cf831d235b0415d40d582ca951c to your computer and use it in GitHub Desktop.

Now let's actually look at the code originally investigated in the author's original video ported to VBA

public sub run()
  while true
    Dim path as string
    while (path = requestedUrls.poll()) != null    
      downloads.add(new DownloadState(path))
    wend
    if not me.connectionDisabled then
      Dim iterator as ListIterator
      set iterator = downloads.listIterator()
      while iterator.hasNext() and not me.connectionDisabled
        Dim download as DownloadState
        set download = iterator.getNext()
        if download.getState() = DownloadState.State.Pending then
          Dim downloader as Download
          set downloader = Download.Create(download.getUrl(), me.downloadDir)
          download.start()
          download.setDownloader(downloader)
          download.moveTo(DownloadState.State.InProgress)
        end if
        if download.getState() = DownloadState.State.InProgress then
          Dim result as DownloadResult
          set result = download.getDownloader().process()
          select case result.getCode()
            case Success
              download.moveTo(DownloadState.State.Complete)
            case InProgress
              'Do nothing
            case Timeout, ConnectionError
              if download.getAttempts() > me.maxAttempts then
                me.connectionDisabled = true
              else 
                download.moveTo(DownloadState.State.InProgress)
              end if
            case HttpError
              Dim httpCode as long
              httpCode = result.getHTTPCode()
              if httpCode = HTTP_REQUEST_TIMEOUT or _ 
                  httpCode = HTTP_BAD_GATEWAY or _ 
                  httpCode = HTTP_SERVICE_UNAVAILABLE or _ 
                  httpCode = HTTP_GATEWAY_TIMEOUT then
                if download.getAttempts() > me.maxAttempts then
                  download.moveTo(DownloadState.State.Complete)
                else
                  download.moveTo(DownloadState.State.InProgress)
                end if
              else
                failedUrls.offer(download.getUrl())
                download.moveTo(DownloadState.State.Complete)
              end if
          end select
        end if
        if download.getState() = DownloadState.State.Complete then 
          iterator.remove()
        end if
      wend
    end if
    if me.connectionDisabled then
      while downloads.size() > 0
        Dim download as DownloadState
        set download = downloads.removeFirst()
        if download.getState() = DownloadState.State.InProgress then
          download.getDownloader().cancel()
        end if
        faildedUrls.offer(download.getUrl())
      wend
    end if
    if downloads.isEmpty() or requestedUrls.isEmpty() then
      DoEvents
    end if
  wend
end sub

The first thing you'll recognise is this is a beast, and quite unusual for something to appear like this in VBA, to be honest. You can definitely see how, in this scenario, the refactoring of the code is definitely worthwhile to make it easier to understand because on the face of it this code is very difficult to comprehend. I have to say I do disagree with how the author refactored the code though. 😅

One thing I notice is the following is in 2 places, but slightly different:

if download.getAttempts() > me.maxAttempts then
  me.connectionDisabled = true
else 
  download.moveTo(DownloadState.State.InProgress)
end if

and later

if download.getAttempts() > me.maxAttempts then
  download.moveTo(DownloadState.State.Complete)
else
  download.moveTo(DownloadState.State.InProgress)
end if

I actually think there is an error here, but I'm going to assume that this is intentional. So this is where we should start in my opinion. I also don't like how me.connectionDisabled can occur in a hidden function in the author's solution, so instead I'll make sure this is explicitely required. Additionally there is no reason to move the download state to InProgress as this is only executed while the state is already InProgress, so instead I'll replace this with just a check for max attempts being reached:

Function maxAttemptsReached(ByRef dl as as DownloadState) as Boolean
  maxAttemptsReached = dl.getAttempts() > me.maxAttempts
End Function

P.S. I also don't know why DownloadState has no Errored value...

I agree with the author that the HTTP code is also a bit of an issue, but I don't agree with removing it all, however checking for timeout, bad gateway etc. could be easily extracted:

Function isUriUnreachable(ByVal result as DownloadResult) as boolean
  Dim httpCode as long
  httpCode = result.getHTTPCode()
  isUriUnreachable = _ 
    httpCode = HTTP_REQUEST_TIMEOUT or _ 
    httpCode = HTTP_BAD_GATEWAY or _ 
    httpCode = HTTP_SERVICE_UNAVAILABLE or _ 
    httpCode = HTTP_GATEWAY_TIMEOUT 
End Function

The original codes HTTP section has some failedUrls value, not sure what it is as it's undefined. Next, we should clarify how DownloadState...Pending is resolved. I'd make a startDownload sub as follows:

Sub startDownload(dl as DownloadState)
  Dim downloader as Download
  set downloader = Download.Create(download.getUrl(), me.downloadDir)
  downloader.start()
  
  dl.setDownloader(downloader)
  dl.moveTo(DownloadState.State.InProgress)
end sub

I'm also going to add a completeDownload sub as follows:

Sub completeDownload(dl as DownloadState)
  dl.moveTo(DownloadState.State.Complete)
End Sub

And finally will split our DownloadState handling code with a select case statement, and move this into a seperate handleDownloads function which returns whether the connection was disabled or not:

Function handleDownloads(ByVal downloads as List) as Boolean
  Dim connectionDisabled as Boolean: connectionDisabled = false
  Dim iterator as ListIterator
  set iterator = downloads.listIterator()
  while iterator.hasNext() and not connectionDisabled
    Dim download as DownloadState
    set download = iterator.getNext()

    select case download.getState() 
      case DownloadState.State.Pending
        Call startDownload(download)
      case DownloadState.State.InProgress
        Dim result as DownloadResult
        set result = download.getDownloader().process()
        select case result.getCode()
          case Success
            Call completeDownload(download)
          case InProgress
            'Do nothing
          case Timeout, ConnectionError
            if maxAttemptsReached(download) then connectionDisabled = true
          case HttpError
            if isUriUnreachable(result) then
              if maxAttemptsReached(download) then Call completeDownload(download)
            else
              failedUrls.offer(download.getUrl())
              Call completeDownload(download)
            end if
        end select
      end if
    end select
    if download.getState() = DownloadState.State.Complete then 
      iterator.remove()
    end if
  wend
  handleDownloads = connectionDisabled
End Function

With another 2 functions/subs to handle getting and clearing downloads, which were also used by the author:

Public Function getDownloads() as List
  set getDownloads = List.Create()
  Dim path as string
  path = requestedUrls.poll()
  While path <> ""
    getDownloads.add(DownloadState.Create(path))
    path = requestedUrls.poll()
  Loop
End Function

Public Sub clearDownloads(downloads as List)
  while downloads.count > 0
    Dim download as DownloadState
    set download = downloads.removeFirst()
    if download.getState() = DownloadState.State.InProgress then
      download.getDownloader().cancel()
    end if
    faildedUrls.offer(download.getUrl())
  wend
End Sub

The run function is now very easy to comprehend.

public sub run()
  while true
    Dim downloads as List
    set downloads = getDownloads()
    if not me.connectionDisabled then _ 
      me.connectionDisabled = handleDownloads(downloads)
    
    if me.connectionDisabled then _ 
      Call clearDownloads(downloads)
    
    if downloads.isEmpty() or requestedUrls.isEmpty() then _ 
      DoEvents
  wend
end sub

Ultimately, the author is not saying don't indent at all. They're saying split code into functions that make sense for what you're trying to achieve. If your just creating a report in VBA, you're unlikely going to run into these kind of situations though, in general.

public sub run()
while true
Dim path as string
while (path = requestedUrls.poll()) != null
downloads.add(new DownloadState(path))
wend
if not me.connectionDisabled then
Dim iterator as ListIterator
set iterator = downloads.listIterator()
while iterator.hasNext() and not me.connectionDisabled
Dim download as DownloadState
set download = iterator.getNext()
if download.getState() = DownloadState.State.Pending then
Dim downloader as Download
set downloader = Download.Create(download.getUrl(), me.downloadDir)
download.start()
download.setDownloader(downloader)
download.moveTo(DownloadState.State.InProgress)
end if
if download.getState() = DownloadState.State.InProgress then
Dim result as DownloadResult
set result = download.getDownloader().process()
select case result.getCode()
case Success
download.moveTo(DownloadState.State.Complete)
case InProgress
'Do nothing
case Timeout, ConnectionError
if download.getAttempts() > me.maxAttempts then
me.connectionDisabled = true
else
download.moveTo(DownloadState.State.InProgress)
end if
case HttpError
Dim httpCode as long
httpCode = result.getHTTPCode()
if httpCode = HTTP_REQUEST_TIMEOUT or _
httpCode = HTTP_BAD_GATEWAY or _
httpCode = HTTP_SERVICE_UNAVAILABLE or _
httpCode = HTTP_GATEWAY_TIMEOUT then
if download.getAttempts() > me.maxAttempts then
download.moveTo(DownloadState.State.Complete)
else
download.moveTo(DownloadState.State.InProgress)
end if
else
failedUrls.offer(download.getUrl())
download.moveTo(DownloadState.State.Complete)
end if
end select
end if
if download.getState() = DownloadState.State.Complete then
iterator.remove()
end if
wend
end if
if me.connectionDisabled then
while downloads.size() > 0
Dim download as DownloadState
set download = downloads.removeFirst()
if download.getState() = DownloadState.State.InProgress then
download.getDownloader().cancel()
end if
faildedUrls.offer(download.getUrl())
wend
end if
if downloads.isEmpty() or requestedUrls.isEmpty() then
End
end if
wend
end sub
Function maxAttemptsReached(ByRef dl as as DownloadState) as Boolean
maxAttemptsReached = dl.getAttempts() > me.maxAttempts
End Function
Function isUriUnreachable(ByVal result as DownloadResult) as boolean
Dim httpCode as long
httpCode = result.getHTTPCode()
isUriUnreachable = _
httpCode = HTTP_REQUEST_TIMEOUT or _
httpCode = HTTP_BAD_GATEWAY or _
httpCode = HTTP_SERVICE_UNAVAILABLE or _
httpCode = HTTP_GATEWAY_TIMEOUT
End Function
Sub startDownload(dl as DownloadState)
Dim downloader as Download
set downloader = Download.Create(download.getUrl(), me.downloadDir)
downloader.start()
dl.setDownloader(downloader)
dl.moveTo(DownloadState.State.InProgress)
end sub
Sub completeDownload(dl as DownloadState)
dl.moveTo(DownloadState.State.Complete)
End Sub
Function handleDownloads(ByVal downloads as List) as Boolean
Dim connectionDisabled as Boolean: connectionDisabled = false
Dim iterator as ListIterator
set iterator = downloads.listIterator()
while iterator.hasNext() and not connectionDisabled
Dim download as DownloadState
set download = iterator.getNext()
select case download.getState()
case DownloadState.State.Pending
Call startDownload(download)
case DownloadState.State.InProgress
Dim result as DownloadResult
set result = download.getDownloader().process()
select case result.getCode()
case Success
Call completeDownload(download)
case InProgress
'Do nothing
case Timeout, ConnectionError
if maxAttemptsReached(download) then connectionDisabled = true
case HttpError
if isUriUnreachable(result) then
if maxAttemptsReached(download) then Call completeDownload(download)
else
failedUrls.offer(download.getUrl())
Call completeDownload(download)
end if
end select
end if
end select
if download.getState() = DownloadState.State.Complete then
iterator.remove()
end if
wend
handleDownloads = connectionDisabled
End Function
Public Function getDownloads() as List
set getDownloads = List.Create()
Dim path as string
path = requestedUrls.poll()
While path <> ""
getDownloads.add(DownloadState.Create(path))
path = requestedUrls.poll()
Loop
End Function
Public Sub clearDownloads(downloads as List)
while downloads.count > 0
Dim download as DownloadState
set download = downloads.removeFirst()
if download.getState() = DownloadState.State.InProgress then
download.getDownloader().cancel()
end if
faildedUrls.offer(download.getUrl())
wend
End Sub
public sub run()
while true
Dim downloads as List
set downloads = getDownloads()
if not me.connectionDisabled then _
me.connectionDisabled = handleDownloads(downloads)
if me.connectionDisabled then _
Call clearDownloads(downloads)
if downloads.isEmpty() and requestedUrls.isEmpty() then End
wend
end sub
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment