Home > Software engineering >  Issues with looping - Need help "refining" my code
Issues with looping - Need help "refining" my code

Time:05-31

I've this ps script that I have written myself. It searches for available Active Directory groups with a keyword, and if no keyword is found, it outputs that it cannot find any AD groups, and asks if I'd like to search again. However, the issue I'm facing, is that it outputs that it cannot find any AD groups, even if it does find groups with my keyword... This be my code:

$group = Read-Host "Which groups would you like to search for?"
$group = $group   "_*"
$groups = Get-ADGroup -Filter {name -like $group} -Properties * | select SAMAccountName, Description
while ($groups -eq $null) {
        if ($groups -eq $null) {
            Write-Verbose "Did not find any matching groups. Try again" -Verbose
            $group = Read-Host "Which groups would you like to search for?"
            $group = $group   "_*"
            Get-ADGroup -Filter {name -like $group} -Properties * | select SAMAccountName, Description   
    } else {
        if ($groups -ne $null) {
            Get-ADGroup -Filter {name -like $group} -Properties * | select SAMAccountName, Description  
        }
    }
}

Now, I know this is probably not that cleanly done, and the formatting could be better... maybe. But I'm having a hard time understanding why it outputs the "try again" message even if results shows up

Thanks!

CodePudding user response:

As commented, the while ($groups -eq $null) already tests for 'nothing found', so you need not test that again inside the loop.

I would suggest a slightly different approach using an endless while loop you break out of if the Get-ADGroup cmdlet returned any results

while ($true) {
    $group = Read-Host "Which groups would you like to search for?"
    # if the user did not enter an empty of whitespace-only string
    if (![string]::IsNullOrWhiteSpace($group)) {
        # Get-ADGroup by default already returns these properties:
        # DistinguishedName, GroupCategory, GroupScope, Name, ObjectClass, ObjectGUID, SamAccountName, SID
        # so in this case, only ask for the extra Description property
        $groups = Get-ADGroup -Filter "Name -like '$($group)_*'" -Properties Description
        if ($groups) {
            # output the two properties
            $groups | Select-Object SamAccountName, Description
            # and exit the while loop
            break
        }
    }
    # once here, you didn't find any groups on the input keyword, so let the user try again
    Write-Verbose "Did not find any matching groups. Try again" -Verbose
}

Note:

  • -Filter should be a string rather than a scriptblock
  • using -Properties * to ask for all is very wasteful if all you eventually care about is two properties.
  • comparing to $null can be done better with if($groups) as opposed to if ($groups -ne $null) and if(!$groups) as opposed to if ($groups -eq $null). If you must compare to $null, then better change the order like if ($null -eq $groups)
  • I used the sub expression operator $() in "Name -like '$($group)_*'", because otherwise, PowerShell will try and expand an undefined variable $groups_. An alternative to this is to use "Name -like '${group}_*'"
  • Related