Make script more cute

Welcome Forums General PowerShell Q&A Make script more cute

This topic contains 5 replies, has 3 voices, and was last updated by

 
Participant
4 years, 2 months ago.

  • Author
    Posts
  • #18829

    Participant
    Points: 0
    Rank: Member

    Hello all

    I'm trying to automatize some things I'm doing in my job and I cmae with couple scripts. One of them is to check who is logged on which machine in domain.

    
    $ADSearchBase = "OU=Computers,DC=domain,DC=local"
    $ADFilter = "*"
    
    Function Get-LoggedUsersOnComputers
        {
        $ADComputersList = Get-ADComputer -SearchBase $ADSearchBase -Filter $ADFilter 
        foreach ($ADComputer in $ADComputersList)
            {
            if ((Test-NetConnection $ADComputer.Name -WarningAction SilentlyContinue).PingSucceeded)
                {
                Write-host -ForegroundColor Green $ADComputer.Name
                $ExplorerProcesses = Get-WmiObject -ComputerName $ADComputer.Name -Class win32_process | where { $_.name -Match "explorer" }
                foreach ($ExplorerProcess in $ExplorerProcesses)
                    {
                    Write-Host -ForegroundColor Yellow " " $ExplorerProcess.GetOwner().User
                    }
                }
            else
                {
                Write-Host -ForegroundColor Red $ADComputer.Name
                }
            }
        }
    
    

    Do you think it can be changed to something more cute or faster? Any professional suggestions?

    Thank you,

  • #18831

    Keymaster
    Points: 1,704
    Helping HandTeam Member
    Rank: Community Hero

    I'm not a huge fan of Write-Host. If the goal is just to produce some indications of success, I'd rely on Write-Verbose. Your function isn't outputting text or an object – it's drawing directly on the screen. So you can't redirect that to a file, or export it to a CSV, or do anything with it. I'd suggest moving to a custom object for the output – it's much more in fitting PowerShell's native patterns.

    I'd use -Filter on Get-WmiObject vs. using Where-Object – better performance to have the filtering happen at the source.

    I'd probably rely on Test-Connection -Quiet vs. the command you're using to do the ping. However, a valid ping doesn't mean WmiObject will succeed. It'd make more sense to just use WmiObject, and trap any errors. A computer can have ICMP blocked, but still allow WMI, and vice-versa.

    Heh, "cuter." 🙂

  • #18858

    Participant
    Points: 0
    Rank: Member

    Is this better?

    $ADSearchBase = "OU=Computers,DC=domain,DC=local"
    $ADFilter = "*"
    
    Function Get-LoggedUsersOnComputers
        {
        $ADComputersList = Get-ADComputer -SearchBase $ADSearchBase -Filter $ADFilter 
        foreach ($ADComputer in $ADComputersList)
            {
            Write-Output $ADComputer.Name
            Try
                {
                $ExplorerProcesses = Get-WmiObject -ComputerName $ADComputer.Name -Class win32_process -Filter "Name='explorer.exe'" -ErrorAction SilentlyContinue
                }
            Finally
                {
                If ($?)
                    {
                    foreach ($ExplorerProcess in $ExplorerProcesses)
                        {
                        Write-Output "  $($ExplorerProcess.GetOwner().User)"
                        }
                    }
                Else
                    {
                    Write-Output "  < << Could not connect. "
                    }
                }
            }
        }
    
    
    Get-LoggedUsersOnComputers
    
  • #18863

    Participant
    Points: 3
    Rank: Member

    Hi. I think that you should also replace the -ErrorAction SilentlyContinue part with -ErrorAction Stop at the Get-WMIObject command in order to catch the error.
    Also instead of using $?, you could put a catch block after try in which you can write the error and use the Continue command to go to the next PC.
    The finally block should contain only the user info; no if statement needed anymore if you use catch.

    • #18864

      Participant
      Points: 0
      Rank: Member

      I had problem with Get-WMIObject cmdlet and catching error. It kinda acts weird way.

  • #18866

    Participant
    Points: 3
    Rank: Member

    In that case you can also remove in my opinion the try and finally keywords because they do not do anything.
    Place the code from those blocks directly in the function.

The topic ‘Make script more cute’ is closed to new replies.