1

I wrote a function to check if a host is online or offline and return $true or $false. This function works perfectly and I would like to improve it a bit by seeing if it's possible to remove the script-wide variables like $Script:arrayCanPingResult and $script:tmpPingCheckServers.

Why do I want this? When I call the function within a foreach loop I usually use the switch -Remember so it doesn't check the same host twice. To be able to use this properly I have to begin all my scripts where I use this function by declaring both variables empty ( $Script:arrayCanPingResult=$script:tmpPingCheckServers=@{}). And I can imagine people forgetting to put this first line in their script and when testing multiple times in the PowerShell ISE editor, it won't do the tests again on the second run when the host has already been checked once in ISE (F5).

Is there a way to avoid using the script-wide variables in this case? So we don't need to declare them empty in the beginning of new scripts? If this was possible, it would be great because then we can include this function in a custom module.

As always, thank you for your advice or help. I really learned a lot here with you guys.

# Function to check if $Server is online
Function Can-Ping ($Server,[switch]$Remember) {

    $PingResult = {
          # Return $true or $false based on the result from script block $PingCheck
          foreach ($_ in $Script:arrayCanPingResult) { 
                   # Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: $_ " -ForegroundColor Green
                   if ($Server -eq $($_.Split(",")[0])) {

                   #Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: We will return $($_.Split(",")[1])" -ForegroundColor Green
                    return $($_.Split(",")[1])  
                   } 
          }
    }

    $PingCheck = {

        $Error.Clear()

        if (Test-Connection -ComputerName $Server -BufferSize 16 -Count 1 -ErrorAction 0 -quiet) { # ErrorAction 0 doesn't display error information when a ping is unsuccessful

            Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: Ping test ok" -ForegroundColor Gray; $Script:arrayCanPingResult+=@("$Server,$true"); return
        } 
        else {
            $Error.Clear()
            Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: Ping test FAILED" -ForegroundColor Gray

            Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: Flushing DNS" -ForegroundColor Gray
            ipconfig /flushdns | Out-Null

            Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: Registering DNS" -ForegroundColor Gray
            ipconfig /registerdns | Out-Null

            Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: NSLookup" -ForegroundColor Gray
            nslookup $Server | Out-Null # Suppressing error here is not possible unless using '2> $null', but if we do this, we don't get $true or $false for the function so '| Out-Null' is an obligation
            if (!$?) {
                Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: NSlookup can't find the host '$Server', DNS issues or hostname incorrect?" -ForegroundColor Yellow
                # Write-Host $Error -ForegroundColor Red
                if ($SendMail) {
                    Send-Mail $MailTo "FAILED Ping test" "$(Get-TimeStamp) NSlookup can't find the host '$Server', hostname incorrect or DNS issues?" "<font color=`"red`">$error</font>"
                }
                $script:arrayCanPingError += "ERROR | $(Get-TimeStamp) Ping test failed: NSlookup can't find the host '$Server', hostname incorrect or DNS issues?$error"
                $script:HTMLarrayCanPingError += "ERROR | $(Get-TimeStamp) Ping test failed:<br>NSlookup can't find the host '$Server', hostname incorrect or DNS issues?<br><font color=`"red`">$error</font>"
                $Script:arrayCanPingResult+=@("$Server,$false")
                return
                }
            else {
                Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: Re-pinging '$Server'" -ForegroundColor Gray
                if (Test-Connection -ComputerName $Server -BufferSize 16 -Count 1 -ErrorAction 0 -Quiet) {
                   Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: Ping test ok, problem resolved" -ForegroundColor Gray
                   $Script:arrayCanPingResult+=@("$Server,$true")
                   return
                }
                else {
                      Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: DNS Resolving is ok but can't connect, server offline?" -ForegroundColor Yellow
                      if ($SendMail) {
                          Send-Mail $MailTo "FAILED Ping test" "$error" "DNS Resolving is ok but can't connect to $Server, server offline?"
                      } 
                      $script:arrayCanPingError += "ERROR Ping test failed: DNS Resolving is ok but can't connect to $Server, server offline?$error"
                      $script:HTMLarrayCanPingError += "ERROR Ping test failed: DNS Resolving is ok but can't connect to $Server, server offline?<br><font color=`"red`">$error</font>"
                      $Script:arrayCanPingResult+=@("$Server,$false")
                      return
                }
            }
        }
    }

    # Call the script block $PingAction every time, unless the switch $Remember is provided, than we only check each server once
    if ($Remember) {
        Write-Host "$(Get-TimeStamp) $Server > Function Can-Ping: Switch '-Remember' detected" -ForegroundColor Gray
        While ($tmpPingCheckServers -notcontains $Server) { 
                  &$PingCheck
                  $script:tmpPingCheckServers = @($tmpPingCheckServers+$Server) #Script wide variable, otherwise it stays empty when we leave the function / @ is used to store it as an Array (table) instead of a string
        }
        &$PingResult
    } 
    else {
          &$PingCheck
          &$PingResult
    }
}
2
  • Short answer: PSCustomObject. I.e. you want to use object as an argument and then return the same object with modified properties. Long answer depends on what your function is actually supposed to do. You call Test-Connection once and the rest is logic (which is too convoluted to even read) and Write-Host calls (Write-Host is evil, avoid it in production code). Could you provide a sample of input and expected output? Commented Jul 20, 2014 at 11:13
  • Thank you for your reply. The function is used like this Can-Ping -Remember SERVER1 and returns $true if SERVER1 is online and $false when it's not. The second time I run Can-Ping -Remember SERVER1 it won't do any tests at all and returns the last known result for that server. In my script I call this function each time within a foreach loop with a new $Server name. Depending on the result I then check PSRemoting capabilities and other stuff. Commented Jul 20, 2014 at 13:56

1 Answer 1

1

Here's how I would do it:

function Get-PingStatus {
    param(
        # Server object.
        [Parameter(Mandatory=$true)]
        [ValidateNotNullOrEmpty()]
        $Server,
        [Parameter(Mandatory=$false)]
        [Bool]$UseLastPingResult = $false
    )

    if ( ($UseLastPingResult) `
    -and (! [String]::IsNullOrEmpty($Server.LastPingResult)) ) {
        # Return unmodified object if LastPingResult property is not empty.
        return $Server 
    }

    try {
        $oPingResult = Test-Connection -ComputerName $Server.Name `
        -BufferSize 16 -Count 1 -ErrorAction Stop

        $Server.LastPingResult = "success"
        # And just in case.
        $Server.IPV4Address = $oPingResult.IPV4Address
    }
    catch {
        $Server.LastPingResult = "failure"
    }

    return $Server
}

Object in, object out. Generally this is the best approach to writing PowerShell functions because: 1) it's in line with what stock cmdlets do and 2) it helps you keep code simple and readable.

Using ErrorAction -Stop with try...catch...finally is also a better idea than ErrorAction -SilentlyContinue with checking some variable afterwards.

Now let's assume that $cServerNames is a collection of server names or anything that can be resolved to IP addresses. Example: @("server1", "server2", "1.2.3.4", "webserver.example.com").

# Converting strings to objects.
$cServers = @()
foreach ($sServerName in $cServerNames) {
    $oServer = New-Object PSObject -Property @{
        "Name" = $sServerName;
        "IPV4Address" = $null;
        "LastPingResult" = $null;
    }

    $cServers += $oServer
}

# Now we can iterate objects and update their properties as necessary.
foreach ($oServer in $cServers) {
    $oServer = Get-PingStatus -Server $oServer -UseLastPingResult

    if ($oServer.LastPingResult -eq "success") {
        # Do something.
    } else {
        # Do something else like an error message.
    }
}

You can add whatever diagnostic output you want but I suggest that you use Write-Output and/or Write-Error instead of Write-Host.

Finally, take note that ping is virtually useless in production code in most cases. It's redundant and proves nothing whether the host replies or not. For example, ping may be OK but then for some reason you get an exception on a subsequent Get-WmiObject query. If you do pings for performance reasons (to save time) you should consider running script blocks in parallel via background jobs or workflows.

Sign up to request clarification or add additional context in comments.

5 Comments

Thank you Alexander. Your post made me rethink my complete setup of $true and $false coding. I've picked this method up from other examples and you are right, it's better to work with objects because it's more flexible. I'll try to change my script to implement the object-way as it seems much better. Thank you again for your help :)
@DarkLite1 You're welcome! ) When I started working with PowerShell I had mostly bash/python background so I also tried to use other things (like hash tables) extensively at first. But then I switched to objects and never looked back.
I've been playing a bit today with your code and I stumbled upon something. The switch LastPingResult is not working as designed (check with verbose). Because it's still doing all the tests every time a duplicate server name is passed. This is probably because of the scope of the function. Maybe it's better to collect the objects outside the function and remove the switch UseLastPingResult entirely, as it isn't doing anything..
It should work. Try using [bool] instead of [switch] maybe. I thought they were the same thing but apparently there is a difference. I need to read up on that.
No problem, it will never work within the function. As when PowerShell leaves the function for the next object it will forget it's variables content from within the function. The best way around this is to create an array outside of the function containing the function's result. I'll try it later on.. Thanks anyway.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.