pdqcom / Bonus-Content

Bonus Content for Webcasts
MIT License
51 stars 19 forks source link

Windows 11 check script - was it written for compatibility with old PS versions? #4

Open LRSFC-DanJ opened 2 years ago

LRSFC-DanJ commented 2 years ago

I got pointed to the Windows 11 check script on here when I was looking for a way to get PDQ Inventory to report on Windows 11 readiness. However I notice some parts of it have been written in what seems like a strange manner compared to Windows PowerShell 5.1 which comes with Windows 10.

I was wondering if this was because the script was written specifically to be compatible with older versions of PowerShell?

Jordan-PDQ commented 2 years ago

I did not write anything specifically for an older version, I was really only thinking of Windows 10. It is absolutely possible I am writing something in an outdated way. I am happy to look into correcting it, are there any specific lines or cmdlets, in particular, that strike you as odd?

LRSFC-DanJ commented 2 years ago

To give some specific examples:

$ProcCompatible = $false
###Compare processor to approved list
foreach($cpu in $final.model){

    if($proc.name -like "*" + $cpu + "*"){
        $ProcCompatible = $true
        break
    }
}

This could be done more efficiently in one line as follows: $ProcCompatible = ($final.model | Where-Object {$_ -match $proc.name}) -ne $null

###Load File into file stream
$file = New-Object System.IO.StreamReader -ArgumentList "C:\dxdiag.txt"

###Setting initial variable state
$Directx12Compatible = $false
$WDDMCompatible = $false
###Reading file line by line
try {
    while ($null -ne ($line = $file.ReadLine())) {
###Mark start of applied policies
        if ($line.contains("DDI Version:") -eq $True) {
            if($line.Trim("DDI Version: ") -ge $DirectxVer){
                $Directx12Compatible = $true
            }
        }
        elseif ($line.contains("Driver Model:") -eq $True) {
            if($line.Trim("Driver Model: WDDM ") -ge $WDDMVer){
                $WDDMCompatible = $true
            }
        }
    }
}

finally {
    $file.Close()
    Remove-Item "C:\dxdiag.txt" -Force
}

This just seems unusual in that it is reading in the file line by line. I would have written:

###Load File (this gets the whole file and puts it into an array of strings, one for each line)
$file = Get-Content "C:\dxdiag.txt"

###Setting initial variable state
$Directx12Compatible = $false
$WDDMCompatible = $false
###Reading file line by line
try {
    foreach ($line in $file) {
###Mark start of applied policies
        $tokens = $line -split " " # splits the line into an array of items that were delimited by spaces
        if ($line.contains("DDI Version:")) {
            if($tokens[2] -ge $DirectxVer){
                $Directx12Compatible = $true
            }
        } elseif ($line.contains("Driver Model:")) {
            if($tokens[3] -ge $WDDMVer){
                $WDDMCompatible = $true
            }
        }
    }
}

finally {
    Remove-Item "C:\dxdiag.txt" -Force
}
Jordan-PDQ commented 2 years ago

I love the first example, that is absolutely a more efficient way to write that. I will work to get the script updated for that. I have found I often lean back on when I was first learning PowerShell and stick with what is familiar to me over what might be best.

For the Get-Content vs StreamReader, that is something I learned from Josh King's presentation on scripting for speed. Get-Content does actually read each line individually and is significantly slower than streamreader. In the case of this script the file from running dxdiag is small enough that the difference is probably not noticeable, but I have tried to default to streamreader for those times I do end up working with larger files.

This Link will give you a breakdown of how Get-Content operates under the hood: https://powershell.org/2013/10/why-get-content-aint-yer-friend/

This Link goes into speed testing the different options: https://posh-able.com/2020/01/12/powershell-performance-part-2-reading-text-files/