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.