Script review

This topic contains 5 replies, has 2 voices, and was last updated by  tommymaynard 1 year, 8 months ago.

  • Author
    Posts
  • #33031

    Graham Beer
    Participant

    Hi,

    I'm writing a script to uninstall windows ADK 8.1 and install ADK 10.

    This function is to uninstall ADK 8.1. I would really value people's opinion on the script and improvements or shortcuts. Thanks !

    #region Uninistall AKD 8.1
    
    Function Uninstall-ADK8.1 { #Step 1 check for existing install of Windows ADK 8.1
    
        [CmdletBinding(SupportsShouldProcess=$True,                    
                       ConfirmImpact='High')]
        Param
        (     
        )
    
        Begin
        { 
        write-verbose "This function will check for a install of Windows ADK 8.1 and unistall if     `nThis is to prepare for Install of Windows 10 ADK"
        }
        Process
        { #Test for existing Install of Windows ADK 8.1
            if (test-path 'C:\Program Files (x86)\Windows Kits\8.1')
            {  
            #Find ADKsetup.exe in C:\ProgramData\Package Cache for uninstall
            $UninstallFolder = dir 'C:\ProgramData\Package Cache\' -Filter "adksetup*" -Recurse 
                if ($UninstallFolder -ne $null) #.Exists gives a boolean result
                    {
                    Write-Verbose "file found"
                    }
               else {
                    Write-Warning "ADKSetup to uninstall ADK 10 not found`n`n Terminating Uninstall process"
                    break
                    }
    
                        #Perform Uninstall
                        Write-Verbose "Windows ADK 10 exists on this machine, will uninstall`n"
                        write-warning ": Uninstalling ADK 8.1 ....."
                                   
                        Start-Process $($UninstallFolder.FullName) -ArgumentList '/quiet /uninstall /norestart' -Wait 
                                 Write-Verbose "Uninstall complete, will clean up any existing folders....."
    
                #Perform check and removal of folder                      
                                #Set folder variables
                                $folder = 'C:\Program Files (x86)\Windows Kits\8.1'
                                $check = $folder | Test-Path
                                #Run Check
                                @{$true = Remove-Item -Path $folder -Recurse -Force -ErrorAction SilentlyContinue; $false = 'Folder does not exist'}[$check -eq $true]
                                ##                          
                                write-verbose "Done`n"  
                    }
                else #No action required
                {
                Write-Verbose "No install of Windows ADK 8.1 in this machine"
                }
        }
        End
        {
        Write-Verbose "Ready for Install of ADK 10"
        }
      }
    #endregion
    
  • #33046

    tommymaynard
    Member

    Here's a start. I've written this out quickly, and so I'll edit this post, and indicate I've done that, if there's more I want to add. Hope this helps!

    1. For completeness, indicate you're using the -Message parameter with Write-Verbose and Write-Warning, -Path with Test-Path, etc. In something that exists longer than a onetime use, such as a script or function, we want to use full cmdlet names and all the parameters, even if a cmdlet's parameter will take a positional value.

    2. Don't use aliases in your code: dir (see above and #6 below).

    3. I once heard someone say to use periods after comments, and even in Write-Verbose and Write-Warning messages. This helps to indicate to the user that everything printed (nothing was cutoff). I do it all the time now.

    4. Do you really need the subexpression operator around $UninstallFolder.FullName? I'd test without it, and remove it if it's not needed. I've only used the subexpression operator when the value is part of a string (inside quotes). It seems it would work fine without it.

    5. If it was me, I wouldn't bother piping $folder to Test-Path. Just use: Test-Path -Path $folder. I don't use pipes unless there's a need, or advantage, in the terms of speed. You only have one object in $folder, so I'd just use my example.

    6. Download PSScriptAnalyzer and test it against your function. It'll help, and probably catch some of what I've mentioned so far.

  • #33048

    Graham Beer
    Participant

    Thanks Tommy, value your input. I'm interesting by point 3,

    "3. I once heard someone say to use periods after comments, and even in Write-Verbose and Write-Warning messages. This helps to indicate to the user that everything printed (nothing was cutoff). I do it all the time now."

    Could you give an example please ?

  • #33049

    tommymaynard
    Member

    This was just to say to end your comments and message with periods. This is up to the script/function author, but I thought I'd include it. I'm out to indicate that it's just like any sentence we write, or read. Without a period, we wouldn't know if the thought was complete, or if the author forgot

    See? ^

    #Perform Uninstall.
    #Perform check and removal of folder.
    Write-Verbose -Message 'No install of Windows ADK 8.1 in this machine.'

    It's not a requirement, but I've started to do it, and thought I'd mention it.

  • #33071

    Graham Beer
    Participant

    Thank you Tommy.
    The PSScriptAnalyzer is that the one on GitHub ?

  • #33092

    tommymaynard
    Member

    Yes, the PSScriptAnalyzer module is available on GitHub.

You must be logged in to reply to this topic.