Construct URLs out of filenames of zero-byte images and open them in Firefox for re-downloading It's a problem with FlashGot: it sometimes download zero-byte files. I think it's because I didn't use something like DownThemAll! to limit the number of concurrent downloads, but I wrote this script anyway to solve my problem.

The script basically sets up a RunspacePool and in each of the spawned thread, checks if either of the constructed Danbooru image URL is valid and appends it to a list that becomes the input to Firefox.

I guess the main problem I encountered was a scoping issue. I can not use custom functions inside a script block unless it's defined in there, which is what I did, or its definition is passed as a parameter to the script block and sourced, which is ugly.

I would be interested in reviews that point out any cargo cult programming, performance quirks and/or violation of "best practices". I'm still a beginner so I'd appreciate more if feedback comes with working code rather than hand-waving.

&lt;#
    .SYNOPSIS
      Opens the Danbooru URLs of zero-byte images in Firefox for re-downloading by FlashGot.
#&gt;

#Requires -Version 3.0
#Requires -Modules Microsoft.PowerShell.Utility, Microsoft.PowerShell.Management, CimCmdlets

Set-StrictMode -Version Latest

$PrepareUrl = {

    param (
        [string]$domain,
        [string]$file
    )

    begin {
        function Get-UrlStatusCode([string]$Url) {
            try {
                [int](Invoke-WebRequest -Uri $Url -UseBasicParsing -DisableKeepAlive -Method Head).StatusCode
            }
            catch [Net.WebException] {
                [int]$_.Exception.Response.StatusCode
            }
        }
    }

    process {
        $url = "$domain/data/$file"
        if ((Get-UrlStatusCode -Url $url) -ne 200) {
            $url = "$domain/cached/data/$file"
            if ((Get-UrlStatusCode -Url $url) -ne 200) {
                $url = $null
            }
        }
        Write-Output $url
    }

}

#region Setting Up Jobs
$domain = 'https://danbooru.donmai.us'
$sourceDir = 'C:\fakepath'
Write-Progress -Activity 'Getting the filenames' -Status "Searching under $sourceDir" -SecondsRemaining -1
filter isEmptyImage {
    if ($_.PSIsContainer -eq $false -and $_.Length -eq 0 -and $_.Name -match '__.+\.(jpg|png|gif)') { $_ }
}
$emptyFiles = Get-ChildItem -Path $sourceDir -Recurse | isEmptyImage | Select-Object -ExpandProperty Name
$NumberOfLogicalProcessors = (Get-CimInstance Win32_Processor).NumberOfLogicalProcessors
$rp = [runspacefactory]::CreateRunspacePool(1, 2 * $NumberOfLogicalProcessors)
$rp.Open()
$jobs = New-Object System.Collections.Generic.List[System.Object]
foreach ($file in $emptyFiles) {
    $job = [powershell]::Create().AddScript($PrepareUrl).AddArgument($domain).AddArgument($file)
    $job.RunspacePool = $rp
    $jobs.Add(
        (New-Object PSObject -Property @{
            Job = $job
            Result = $job.BeginInvoke()
        })
    )
}
#endregion
#region Collecting Jobs
$urlList = New-Object System.Collections.Generic.List[System.Object]
$jobCount = $jobs.Count
while ($jobs.Count -gt 0) {
    $WriteProgressParams = @{
        Activity = 'Waiting for jobs to complete'
        Status = "$($jobs.Count) jobs remaining"
        PercentComplete = (($jobCount - $jobs.Count) / $jobCount * 100)
    }
    Write-Progress @WriteProgressParams # splatting
    Start-Sleep -Milliseconds 50
    foreach ($job in $($jobs | Where-Object { $_.Result.IsCompleted -eq $true })) {
        $result = [string]$job.Job.EndInvoke($job.Result)
        if ($result) {
            $urlList.Add($result)
        }
        $job.Job.Dispose()
        [void]$jobs.Remove($job)
    }
}
#endregion
if ($urlList) {
    Start-Process -FilePath 'C:\Program Files\Mozilla Firefox\firefox.exe' -ArgumentList $urlList
}
$rp.Close()
$rp.Dispose()

Set-StrictMode -Off
I don't see anything really wrong with your script. It looks quite functional, and I don't see anything superfluous, though it is very verbose. If it weren't for the fact that I don't see a ; at the end of every line I'd assume you're more accustomed to C# than PowerShell from how structured your script is, not that there's anything wrong with that at all.

All that said, if it were me I think I'd have done a couple of things differently. Nested If statements are hard to read in my opinion. I would prefer to use a Switch statement instead to define cases.

process {
    Switch ($true){
        {(Get-UrlStatusCode -Url "$domain/data/$file") -ne 200} {"$domain/data/$file";continue}
        {(Get-UrlStatusCode -Url "$domain/cached/data/$file") -ne 200} {"$domain/cached/data/$file";continue}
        default {$null}
    }
}


You may note that I output the strings directly in the case scriptblocks; that is because Write-Output is implied, and everything not otherwise directed (such as capturing it in a variable, or using Write-Host or Out-File which redirect the output to the screen or a file respectively) is returned from a function to the pipeline.

For your Get-ChildItem command I would suggest using the -file parameter. That way the FileSystem provider only returns files to PowerShell and you don't have to filter out folder later. This will help speed things up in general, and also simplifies your filter. Kudos on using a filter by the way, almost nobody does it and it's really under appreciated IMHO. Along those same lines I would strongly suggest letting the provider filter files as well using -include, then passing it to your filter. It would look something like this:

filter isEmptyImage {
    if ($_.Length -eq 0 -and $_.Name -match '__.+\.(jpg|png|gif)') { $_ }
}
$emptyFiles = Get-ChildItem -Path $sourceDir -Recurse -Include '*.jpg','*.png','*.gif' -File | isEmptyImage | Select-Object -ExpandProperty Name


This way the File System provider only returns files that end in .jpg, .png, and .gif, and PowerShell has less to sort through.

Lastly, PowerShell is pretty bad about adding and removing things from arrays. It rebuilds its array each time, so I think I would avoid that by capturing the jobs all at once, and then changing the While clause to be a little more intrusive.

$jobs = foreach ($file in $emptyFiles) {
    $job = [powershell]::Create().AddScript($PrepareUrl).AddParameters(@{domain=$domain;file=$file})
    $job.RunspacePool = $rp
    [PSCustomObject]@{
        Job = $job
        Result = $job.BeginInvoke()
    }
}


Or if you want to get fancy you could simply do:

$Jobs = $emptyFiles | %{ [powershell]::Create().AddScript($PrepareUrl).AddParameters(@{domain=$domain;file=$_}) | %{ $_.RunspacePool = $rp;$_ } } | Select *,@{l='Result';e={$_.Job.BeginInvoke()}}


Then we change over to a Do/While loop instead of just a While loop:

$jobsLeft = $jobs | Where-Object { $_.Result.IsCompleted }
$urlList = Do {
    $jobsLeft = $jobs | Where-Object { $_.Result.IsCompleted }
    $WriteProgressParams = @{
        Activity = 'Waiting for jobs to complete'
        Status = "$($jobsLeft.Count) jobs remaining"
        PercentComplete = (($jobs.Count - $jobsLeft.Count) / $jobs.Count * 100)
    }
    Write-Progress @WriteProgressParams # splatting
    Start-Sleep -Milliseconds 50
    foreach ($job in $jobsLeft) {
        [string]$job.Job.EndInvoke($job.Result)|?{$_}
        [void]$job.Job.Dispose()
        [void]$jobs.Remove($job)
    }
} while ($jobsLeft.Count -gt 0)


Those are my suggestions, take'em or leave'em.