Make script more cute

This topic contains 5 replies, has 3 voices, and was last updated by Profile photo of adi dumitras adi dumitras 2 years, 2 months ago.

  • Author
    Posts
  • #18829
    Profile photo of Ondrej Zilinec
    Ondrej Zilinec
    Participant

    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
    Profile photo of Don Jones
    Don Jones
    Keymaster

    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
    Profile photo of Ondrej Zilinec
    Ondrej Zilinec
    Participant

    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
    Profile photo of adi dumitras
    adi dumitras
    Participant

    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
      Profile photo of Ondrej Zilinec
      Ondrej Zilinec
      Participant

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

  • #18866
    Profile photo of adi dumitras
    adi dumitras
    Participant

    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.

You must be logged in to reply to this topic.