3

I have 2 functions written in PowerShell 5. Each has code that is identical at the top and bottom but the code in the middle varies. Like this simplified version (but more complex in practice there are 9 of them not 2).

function Get-PingData {
    param(
        [string[]]$computers
    )
    
    # Generic stuff
    Write-Host "Starting..."

    # Specific stuff
    $resultsArray = @()
    foreach( $computer in $computers ) {
        $result = Test-Connection -ComputerName $computer -Quiet
        $resultsArray += $result
    }

    # Generic stuff
    $output = ( $resultsArray -join "," )
    Write-Host "Done!"
    $output
}

function Test-TCPConnection {
    param(
        [string[]]$computers
    )
    
    # Generic stuff
    Write-Host "Starting..."

    # Specific stuff
    $resultsArray = @()
    foreach( $computer in $computers ) {
        $result = ( Test-NetConnection -Port 3389 -ComputerName $computer ).TcpTestSucceeded
        $resultsArray += $result
    }

    # Generic stuff
    $output = ( $resultsArray -join "," )
    Write-Host "Done!"
    $output
}


$servers = @("server1", "server2")
Get-PingData $servers
Test-TCPConnection $servers

The code works but there is much commonality between them. So, I decided to make the function generic and pass a code block each time, like this:

function Get-GenericStuff {
    param(
        [string[]]$computers,
        [scriptblock]$sb
    )
    
    # Generic stuff
    Write-Host "Starting..."

    # Specific stuff
    $resultsArray = @()
    foreach( $computer in $computers ) {
        $result = $sb.Invoke()
        $resultsArray += $result
    }

    # Generic stuff
    $output = ( $resultsArray -join "," )
    Write-Host "Done!"
    $output
}


$servers = @("server1", "server2")
$codeblock = { Test-Connection -ComputerName $computer -Quiet }
Get-GenericStuff $servers $codeblock
$codeblock = { ( Test-NetConnection -Port 3389 -ComputerName $computer ).TcpTestSucceeded }
Get-GenericStuff $servers $codeblock

This is much shorter and also works. However, I am very uncomfortable with the fact that the codeblocks include a variable name ("$computer") that is used inside the function: if the function decides, for example, to change the variable name from $computer to $server, the codeblock also needs to be changed and this makes the code liable to break in the future.

This leads me to think that I am going about the problem incorrectly. Is there a better way to do this such that I maintain a single generic function but also avoid this brittleness?

2

2 Answers 2

4
  • Choosing $_ as the name of the variable in the input script blocks that will receive input from your Get-GenericStuff wrapper function is indeed a good choice, given the established semantics of the automatic $_ variable.

  • However, I suggest avoiding the .Invoke() method to execute script blocks in PowerShell , because - compared to invocation via &, the call operator, and invocation by cmdlets - it changes the semantics of the call in several respects - see this answer for more information.

Invoking a given script block that references $_ with ForEach-Object is therefore the better choice: It automatically passes its pipeline input objects to a given script block one by one, bound to $_

A simplified example:

function Get-GenericStuff {
  param(
    [string[]] $ComputerName,
    [scriptblock] $ScriptBlock
  )
  
  # Generic stuff
  Write-Host "Starting..."

  # Specific stuff
  # NOTE: Use of ForEach-Object ensures that each pipeline input
  #       object is bound to $_, as seen inside script block $sb
  $ComputerName | ForEach-Object $ScriptBlock

  # Generic stuff
  Write-Host "Done!"
}

# Sample call
Get-GenericStuff -ComputerName foo, bar -ScriptBlock { "[$_]" }

Output:

Starting...
[foo]
[bar]
Done!

However, note that this approach invariably passes the specified computer names one by one to the specified script block resulting in synchronous, sequential execution.

This forgoes the potential for parallel execution, which is typically built into cmdlets that accept a -ComputerName parameter, such as Test-Connection[1]

You can address this via an (by definition optional) -AsArray switch that instructs the wrapper function to pass the given computer names as a single array argument:

function Get-GenericStuff {
  param(
    [string[]] $ComputerName,
    [scriptblock] $ScriptBlock,
    [switch] $WithArray
  )

  # Generic stuff
  Write-Host "Starting..."

  # Specific stuff
  if ($WithArray) { # Pass the $ComputerName array *as as whole*
    ForEach-Object $ScriptBlock -InputObject $ComputerName
  } else {
    # NOTE: Use of ForEach-Object ensures that each pipeline input
    #       object is bound to $_, as seen inside script block $sb
    $ComputerName | ForEach-Object $ScriptBlock
  }

  # Generic stuff
  Write-Host "Done!"
}

# Sample call
Get-GenericStuff -WithArray -ComputerName foo, bar -ScriptBlock { "[$_]" }

Output:

Starting...
[foo bar]
Done!

Note how [foo bar] implies that array foo, bar was passed as a whole; it is the equivalent of "[$('foo', 'bar')]"


Finally, note that PowerShell (Core) 7+ introduced general, thread-based parallelism via ForEach-Object's -Parallel parameter.

Thus, you could extend the function with a -Parallel switch (that is mutually exclusive with -WithArray):

function Get-GenericStuff {
  param(
      [string[]] $ComputerName,
      [scriptblock] $ScriptBlock,
      [Parameter(ParameterSetName='WithArray')]
      [switch] $WithArray,
      [Parameter(ParameterSetName='Parallel')]
      [switch] $Parallel
  )

  # Generic stuff
  Write-Host "Starting..."

  # Specific stuff
  if ($WithArray) { # Pass the $ComputerName array *as as whole*
    ForEach-Object $ScriptBlock -InputObject $ComputerName
  } elseif ($Parallel) {
    # Pass each element of the $ComputerName array to a *separate thread*.
    $ComputerName | ForEach-Object -Parallel $ScriptBlock
  } else {
    # NOTE: Use of ForEach-Object ensures that each pipeline input
    #       object is bound to $_, as seen inside script block $sb
    $ComputerName | ForEach-Object $ScriptBlock
  }

  # Generic stuff
  Write-Host "Done!"
}

# Sample call
Get-GenericStuff -Parallel -ComputerName foo, bar -ScriptBlock { "[$_]" }

[1] In this specific case, -ComputerName happens to be an alias of -TargetName.

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

2 Comments

edit I've now digested this. 1. Yes, I see switching to $_ is preferable. 2. Re not using .Invoke() - oh dear, I think I need to go back an re-write some of my older scripts. ;-) But thanks for pointing that out and the reference, I've noted it. 3. Re using -WithArray, appreciated. Once I get this bit working, parallelising it was going to be the next step. 4. Re -Parallel - yes I read about this (not used it yet), but unfortunately, with this one I am workng with Windows PowerShell (not Core) only. :-(
Thanks for the feedback, @Mark: .Invoke() may often not be a problem in practice, but using & or . (or, as in this case, ForEach-Object) not only avoids different behavior in edge cases, but is simpler. Re ForEach-Object -Parallel: while a handy option in general, for network/remoting commands with an (array-typed) -ComputerName parameter it is better to use the suggested -WithArray approach, even in PowerShell 7+
2

You can use .InvokeWithContext with $_ ($PSItem) as the defined context variable:

function Get-GenericStuff {
    param(
        [Parameter(Mandatory)]
        [string[]] $computers,

        [Parameter(Mandatory)]
        [scriptblock] $sb
    )

    # Generic stuff
    Write-Host 'Starting...'

    # Specific stuff
    foreach ($computer in $computers) {
        [pscustomobject]@{
            Computer = $computer
            Result   = $sb.InvokeWithContext($null, [psvariable]::new('_', $computer))[0]
        }
    }
}


$servers = @('google.com', 'stackoverflow.com')
$codeblock = { Test-Connection -ComputerName $_ -Quiet }
Get-GenericStuff $servers $codeblock | Out-Host
$codeblock = { ( Test-NetConnection -Port 80 -ComputerName $_ ).TcpTestSucceeded }
Get-GenericStuff $servers $codeblock | Out-Host

The defined variable can be literally anything you would like, $_ is a commonly used / known variable but it could mind as well be this:

$sb.InvokeWithContext($null, [psvariable]::new('this', $computer))[0]

Then in the Script Block:

{ Test-Connection -ComputerName $this -Quiet }

3 Comments

Thank you - I'll take a look at this today as well.
I'm assuming that InvokeWithContext has the same disadvantages as linked to by @mklement0? (In other words, better to use '&' ?)
@Mark I don't see a disadvantage with using .InvokeWithContext(..) for invoking single commands like you're doing

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.