Script adding +1

This topic contains 18 replies, has 5 voices, and was last updated by  Graham Beer 1 year, 11 months ago.

  • Author
    Posts
  • #31062

    Graham Beer
    Participant

    Sorry, not a great description but tricky to sum up !
    I have the below script, so when i select the "option" it will uninstall the app via the string. (As you can see its a working progress).

    If i choose 7 for example it adds 1 to it and gives me the option for 8 !
    The only way to get round this is do $a = [int]($a – '1')

    Is this my best way around or am i missing something ?

    function Uninstall-Applications
    {
         param (
         [string]$Title = 'Uninstall Application List',
         [regex]$Criteria = '^[0-99]$'
    
         )
         Clear-host
    
         Write-host -ForegroundColor Yellow " Please select index number for application to Uninstall : "
         Write-Host -ForegroundColor Yellow " Q: Press 'Q' to quit."
         $table | ft -AutoSize
    
    
    try {
        $32Uninstall = Get-ItemProperty HKLM:\Software\Microsoft\Windows\CurrentVersion\Uninstall\* 
        } 
    Catch {
        write-error $_.exception.message
        }
    
    Try {
    $Table = $32Uninstall.displayname | 
    ForEach-Object { $option = 1 } {
        
        [PSCustomObject] @{ Option = $option; '-' = ":" ;Displayname = $_ } ;
        $option++
        }    
       }
        Catch {
        write-error $_.exception.message
       } 
    }
    
    Write-Host "================ $Title ================"
    #do
    
         Uninstall-Applications
         $a = Read-Host "Please make a selection"
        
      if ([Regex]::IsMatch($a,$Criteria)) {
      $a = [int]($a -'1')
      $Table[$a] }
      else {
      write-output "invalid selection" }
       
    #until ($input -eq 'q')
    
  • #31064

    Bob McCoy
    Participant

    First things first ...
    What are you attempting with ...

    [regex]$Criteria = '^[0-99]$'
    

    And why do you have the Title in the param block?

  • #31065

    Graham Beer
    Participant

    ok, so the results that come back from the uninstall (generally most people don't have 99 apps ! ) and a number between 1 and 99 must be selected.

  • #31067

    Bob McCoy
    Participant

    The Regex is wrong for what you want to do. And at this point I think the param block needs to go away.

    Do you always plan on running this in interactive mode where your asking a user to make a selection?

  • #31068

    Graham Beer
    Participant

    It's a tool for desktop engineers and not users. To develop my piwershell skills I've decided to make some tools to aid our desktop guys.

  • #31077

    Graham Beer
    Participant

    So some further playing and this works but still adds the +1 to the selection. Must be to do with the $option++...

    $Uninstall =  Get-ItemProperty HKLM:\Software\Microsoft\Windows\CurrentVersion\Uninstall\* |
     
         ForEach-Object { $option = 1 } {
     [PSCustomObject] @{ Option = $option; '-' = ":" ;Displayname = $_.displayname;Uninstall = $_.UninstallString } ;
        $option++
        } 
    
    $Uninstall | select Option, Displayname | ft -AutoSize
      
       $a = Read-Host "Please make a selection" 
    
       $uninstall[$a] | select Displayname, @{name="Uninstallstring";expression={$_.Uninstall}} -OutVariable Selection
       "`n"
       "string is: "
    
       $selection.Uninstallstring
    

    Any thoughts ?

  • #31098

    Mark
    Participant

    I don't know if it's the code or my ISE environment, but when I run it, there is no title and $Table is blank. Since $Title and $Table exist only within the bounds of the function, they don't exist in the main script.

    I also don't see the list displayed when running the script.

    I'd also change some of the logic when building $Table:
    $script:Table = $32Uninstall.displayname |
    ForEach-Object { $option = 0 } {
    $option++
    [PSCustomObject] @{ Option = $option; '-' = ":" ;Displayname = $_ } ;

    This avoids $option's value from being greater than the actual number of options.

  • #31101

    Graham Beer
    Participant

    Have you run
    $Uninstall = Get-ItemProperty HKLM:\Software\Microsoft\Windows\CurrentVersion\Uninstall ?

    (sorry for the daft question)

  • #31102

    Graham Beer
    Participant

    Is this a local scope $script:Table ?

  • #31107

    Mark
    Participant

    The original script at the top of the posting simply had "$table" being populated.

    When I ran the script, $table has data while in the Uninstall-Applications function but not in the mainline code.

    I recommended the change to "$script:Table" to allow the variable to exist outside of the function.

  • #31108

    Mark
    Participant

    Ahh, if it was a snake, it would have bit me.

    $table is zero based. Option 1 is stored in $table[0] and option 7 is stored in $table[6]. So $table[7] contains option 8

    Yes, you will have to subtract one.

  • #31109

    Graham Beer
    Participant

    Thanks Mark, appreciate your help.

  • #31139

    Max Kozlov
    Participant

    The overall script conception unclear and against powershell rules
    Can I suggest my variant ?

    function Get-Applications {
        Get-ItemProperty HKLM:\Software\Microsoft\Windows\CurrentVersion\Uninstall\* 
    	# here you can filter out some applications
    }
    
    function Show-ApplicationsForUninstall($applist) {
    	$number = 1;
    	Write-Host 'Uninstall Application List:'
    	foreach($app in $applist) {
    		Write-Host ('{0} - {1}' -f $number, $app.DisplayName)
    		$number++
    	}
    }
    
    function Get-AppNumberFromUser($maxcount) {
    	Write-host -ForegroundColor Yellow " Please select index number for application to Uninstall [1..$maxcount]: "
    	Write-Host -ForegroundColor Yellow " Q: Press 'Q' to quit."
    	do {
    		$userinput = Read-Host "Please make a selection"
    	} until ($userinput -eq 'Q' -or ($userinput -match '^\d+$' -and [int]$userinput -ge 1 -and [int]$userinput -le $maxcount));
    	# first match 'Q', next match integer input, last match >=1 and < =maxcount
    	$userinput
    }
    
    function Uninstall-Application($app) {
    	Write-Host ('Uninstalling app {0}' -f $app.DisplayName)
    	#uninstall logic here
    }
    
    #Main code flow
    
    $Applications = Get-Applications
    
    Show-ApplicationsForUninstall $Applications
    
    $appnum = Get-AppNumberFromUser($Applications.Count)
    
    if ($appnum -ne 'Q') {
    	Uninstall-Application $Applications[$appnum-1] # arrays are zero based
    }
    
  • #31187

    Graham Beer
    Participant

    Max, that looks good. Yours does seem better, I was along way off 🙁

  • #31188

    Graham Beer
    Participant

    So what are the powershell rules? Put everything in functions and call them at the end ? Is there and guidelines to help me ?

  • #31191

    Mark
    Participant

    Functions need to be defined before they can be called.

    So the function definitions go before any running code.

    I typically put my "running code" in a function at the top of the script, put all the other functions after that.

    At the very bottom, I call the function with my running code.

    YTMV (your tastes may vary)

  • #31194

    Graham Beer
    Participant

    My code was suggested to not conform to powershell rules...

  • #31232

    Max Kozlov
    Participant

    1. verb-noun notation
    2. get function should get something, show function show something and so on 🙂
    3. minimize usage of write-host
    4. user interaction implemented as separate code (if it not standard confirmation)

    and it not only powershell, but universal good coding style
    this add a lot reusability and show clearly what you do, especially if you read your code after few years 🙂

    for example if you not need uninstall but installation report you can
    1. rewrite completely, using copy paste and catch bugs because you use global variables or multipurpose functions
    2. import-module myapplicationmanagement ; get-applications | export-csv , get-applications | export-html and even
    send-mailmessage -body (get-applications | select displayname, installationdate )

    I don't remember where I read rules all compiled, may be in some PS book, but it for your own convenience 🙂

  • #31233

    Graham Beer
    Participant

    Thanks Max, that's very useful. Appreciate it.

You must be logged in to reply to this topic.