I'm an absolute newb when it comes to PowerShell, and this is pretty much my first real script. I'd now like to know ways to improve it and if possible to shorten the code even further. I use it together with an XML file containing links and keywords to download the "latest" version of different applications to keep my deploymentshare up-to-date.

##################################################################################################################
# Set variables
##################################################################################################################

$DownloadFolder = "$PSScriptRoot\Downloads"
$ApplicationShare = "E:\MDTProduction\Applications"$Time = (Get-Date -Format 'yyyy-MM-dd_HH_mm_ss')
$LogPath = "$PSScriptRoot\DownloadLog_$Time.txt" ################################################################################################################## # Function to create download folder ################################################################################################################## function CreateDownloadFolder () { try { if (Test-Path "$FinalDownloadFolder")
{
Write-Host "$Step1$Skip" -ForegroundColor Yellow
}
else
{
New-Item "$FinalDownloadFolder" -ItemType Directory -ErrorAction Stop | Out-Null Write-Host "$Step1$Done" -ForegroundColor Green } } catch {$ErrorMessage = $_.Exception.Message Write-Host "$Step1$Fail" -ForegroundColor Red Write-Information "Failed to create folder with error message:$ErrorMessage" .
break
}
}

##################################################################################################################
##################################################################################################################

{
try
{
Write-Progress -Activity "Downloading $FileName" -Status "Please wait..." Invoke-WebRequest$DownloadURL -OutFile $FinalDownloadFolder$Filename -ErrorAction Stop Write-Host "Step2Done" -ForegroundColor Green } catch { ErrorMessage = _.Exception.Message Write-Information "Failed to download application with error message: ErrorMessage" . Write-Host "Step2Fail" -ForegroundColor Red break } } ################################################################################################################## # Function to compare file versions and replace files ################################################################################################################## function CompareFilesAndReplace () { try { NewFile = (Get-Item FinalDownloadFolder$Filename -ErrorAction SilentlyContinue)$OldFile = (Get-Item $FinalApplicationShare\*.* -Exclude *.mst, *.ini -ErrorAction SilentlyContinue)$NewFileVersion = $NewFile.VersionInfo.FileVersion -replace ",","." -replace " ",""$OldFileVersion = $OldFile.VersionInfo.FileVersion -replace ",","." -replace " ","" if ($OldFile -and $NewFile) { if ($NewFileVersion -gt $OldFileVersion) { Write-Host "$Step3$Done" -ForegroundColor Green Remove-Item "$OldFile" -Force -ErrorAction Stop
Copy-Item "$NewFile" -Destination "$FinalApplicationShare" -Force -ErrorAction Stop
Write-Host "$Step4$Done" -ForegroundColor Green
Write-Host "INFO: $NewFileVersion >$OldFileVersion" -ForegroundColor Cyan
Write-Host "INFO: An update was found, files were replaced..." -ForegroundColor Cyan
}
elseif ($NewFileVersion -eq "" -and$OldFileVersion -eq "")
{
Write-Host "$Step3$Done" -ForegroundColor Green
Remove-Item "$OldFile" -Force -ErrorAction Stop Copy-Item "$NewFile" -Destination "$FinalApplicationShare" -Force -ErrorAction Stop Write-Host "$Step4$Done" -ForegroundColor Green Write-Host "INFO: No version found, took default action to replace files." -ForegroundColor Cyan } elseif ($NewFileVersion -eq $OldFileVersion) { Write-Host "$Step3$Done" -ForegroundColor Green Write-Host "INFO: Downloaded version$NewFileVersion, current version $OldFileVersion" -ForegroundColor Cyan Write-Host "INFO: File versions are equal, keeping existing file." -ForegroundColor Cyan } } else { Write-Host "INFO: Running instance in download only." -ForegroundColor Cyan Write-Information "Unable to find any files in the$FinalApplicationShare directory, if this is supposed to be true disregard this message, otherwise check the path..."
}
}
catch
{
$ErrorMessage =$_.Exception.Message
Write-Information "Failed to query or replace application with error message: $ErrorMessage" . Write-Host "$Step3$Fail" -ForegroundColor Red break } } ################################################################################################################## # Begin processing the XML-data and start up the main functions of the script ################################################################################################################## Start-Transcript -Path "$LogPath" -Force

#Import XML-data
try
{
[xml]$DownloadSettings = Get-Content "$PSScriptRoot\DownloadSettings.xml"
}
catch
{
$ErrorMessage =$_.Exception.Message
Write-Host "Failed to get settings from $DownloadSettings with error message:$ErrorMessage" .

break
}

#Awesome special effects
$Step1 = "STEP: Create the download folder..."$Step2 = "STEP: Download the application... "
$Step3 = "STEP: Compare the applications... "$Step4 = "STEP: Replace the applications... "
$Done = "ttttttt [DONE]"$Fail = "ttttttt [FAIL]"
$Skip = "ttttttt [SKIP]" #Process XML-data foreach ($Application in $DownloadSettings.xml.Application) {$DownloadURL = $Application.DownloadURL$FolderName = $Application.ApplicationName$Filename = $Application.Filename$Keyword = $Application.Keyword$FinalDownloadFolder = "$DownloadFolder$Foldername\Source" FinalApplicationShare ="ApplicationShare$Foldername\Source" #Use these settings if a link is present and keyword and filename are not set if (($DownloadURL) -and (!$Keyword) -and (!$Filename))
{
$Block = "[1]"$FileName = $DownloadURL.Split('/')[-1].Split('?')[0] -replace "%20","_" } #Use these settings if link and keyword has a value, but filename is not set elseif (($DownloadURL) -and ($Keyword) -and (!$Filename))
{
$Block = "[2]"$DownloadURL = ((Invoke-WebRequest $DownloadURL).Links | Where {$_.href -like "*http*$Keyword*" }).href$Filename = $DownloadURL.Split('/')[-1].Split('?')[0] -replace "%20","_" } #Use these settings if link, keyword and filename all have values elseif (($DownloadURL) -and ($Keyword) -and ($Filename))
{
$Block = "[3]"$DownloadURL = ((Invoke-WebRequest $DownloadURL).Links | Where {$_.href -like "*http*$Keyword" -or$_.innerText -like "$Keyword" }).href } #Output some useful information to the logs, in case of troubleshooting Write-Host "" Write-Host "$Foldername" -ForegroundColor White;
Write-Host "-------------------------------------------------------------------"
Write-Information "ConditionBlock.: $Block" Write-Information "DeploymentShare:$FinalApplicationShare"
Write-Information "DownloadFolder.: $FinalDownloadFolder" Write-Information "DownloadURL....:$DownloadURL"
Write-Information "Filename.......: $Filename" Write-Information "Keyword........:$Keyword"

CompareFilesAndReplace
}

Write-Host
Stop-Transcript


Sample input XML file used:

<xml>

<Application>
<ApplicationName>Install - 7-Zip (Latest) - x64</ApplicationName>
<FileName></FileName>
<Keyword>*7z*x64*exe*</Keyword>
</Application>

<Application>
<ApplicationName>Install - Adobe Flash Player (Latest) - x86</ApplicationName>
<FileName></FileName>
<Keyword></Keyword>
</Application>

<Application>
<ApplicationName>Install - Adobe Flash Player for IE (Latest) - x86</ApplicationName>
<FileName></FileName>
<Keyword>*flashplayer*exe*</Keyword>
</Application>

<Application>
<ApplicationName>Install - Citrix Receiver (Latest) - x86</ApplicationName>
<FileName></FileName>
<Keyword></Keyword>
</Application>

<Application>
<ApplicationName>Install - FileZilla (Latest) - x64</ApplicationName>
<FileName></FileName>
<Keyword>*filezilla*64*exe*</Keyword>
</Application>

<Application>
<ApplicationName>Install - Google Chrome (Latest) - x64</ApplicationName>
<FileName></FileName>
<Keyword></Keyword>
</Application>

<Application>
<ApplicationName>Install - Java Runtime (Latest) - x86-x64</ApplicationName>
<FileName>Java_x64_latest.exe</FileName>
<Keyword>*windows*offline*64*bit*</Keyword>
</Application>

<Application>
<ApplicationName>Install - Java Runtime (Latest) - x86-x64</ApplicationName>
<FileName>Java_x86_latest.exe</FileName>
<Keyword>*windows*offline</Keyword>
</Application>

<Application>
<ApplicationName>Install - KeePass (Latest) - x86</ApplicationName>
<FileName></FileName>
<Keyword>*keepass*setup*exe*</Keyword>
</Application>

<Application>
<ApplicationName>Install - Mozilla FireFox (Latest) - x64</ApplicationName>
<FileName></FileName>
<Keyword>*win64*firefox*exe*</Keyword>
</Application>

<Application>
<ApplicationName>Install - Notepad++ (Latest) - x64</ApplicationName>
<FileName></FileName>
<Keyword>*npp*installer*64*</Keyword>
</Application>

<Application>
<ApplicationName>Install - TeamViewer (Latest) - x86</ApplicationName>
<FileName></FileName>
<Keyword></Keyword>
</Application>

</xml>


Good use of comments and I see some very verbose output being used here. All in all the script is built well. That being said there are a few points worth bringing up. Some carry more weight then others.

You are using parent variables a lot in this code. While that is perfectly acceptable and that is by design how PowerShell works. Quoting about_scopes

Unless an item is private, the child scope can view the items in the parent scope. And, it can change the items by explicitly specifying the parent scope, but the items are not part of the child scope.

It is not initially clear where the variables you are using are declared. You have some at the start of the script, before your XML loop and inside the loop. If you are going to be using these try and at least keep them together where possible. Better yet....

## Function parameters

Your functions could easily be used in other scripts or endeavors. However, they currently are not very scalable with their variables that rely on parent scope. I would look into your functions taking parameters for input. A more in depth start would be to look at [about_functions][2] but a simple example would be for DownloadFiles:

function DownloadFiles{
param(
[string]$DownloadUrl, [string]$Destination
)

# Stuff goes here
}


DownloadFiles -DownloadUrl $DownloadURL -Destination "$FinalDownloadFolder$Filename"  ## Error handling You do this very well. Especially when it comes to downloading files and verifying if your XML attributes are populated. You do miss this two places where you are doing some initial downloads though DownloadURL = ((Invoke-WebRequest DownloadURL).Links | Where { _.href - # ... truncated DownloadURL = ((Invoke-WebRequest DownloadURL).Links | Where { _.href -  Those could fail just as easily and the script would fall flat. You should account for those as well. # String Concatenation for Creating File Paths  FinalDownloadFolder = "DownloadFolder$Foldername\Source"


You gather both parent folder and filename separately and put them together later. Consider using the [io.path] method Combine() which will make it so you don't have to worry about the presence of a slash in the input.

Write-Host "Failed to get settings from $DownloadSettings with error message:$($_.Exception.Message)"  The sub-expression will allow PowerShell to expand that variable inside the string. ## Truthy Falsy consistency You check variables as booleans which is perfectly fine. I see you using two different styles when you are doing it though if ($NewFileVersion -gt $OldFileVersion)  and if (($DownloadURL) -and (!$Keyword) -and (!$Filename))
`

You don't need to use the brackets in that second case. Just remove them. While they do have there place they are redundant in the above example.

• Thanks @Matt, this was exactly the kind of help I was looking for. I updated the script, did I get it right? – Dalle Sep 30 '16 at 18:22