Null output from nested function defined in Begin block

This topic contains 8 replies, has 3 voices, and was last updated by Profile photo of Tony Pagliaro Tony Pagliaro 8 months ago.

Viewing 9 posts - 1 through 9 (of 9 total)
  • Author
    Posts
  • #34280
    Profile photo of Tony Pagliaro
    Tony Pagliaro
    Participant

    Hi,

    I have a cmdlet I'm building with a simple function defined in the Begin block (so that I do not have to keep repeating the same code throughout the process block). It references the single value from the group of values from the foreach loop in the Process block. Here's a dumbed-down version that produces the same issue (null output).

    function test-begin
    {
        [CmdletBinding()]
        Param
        (
            [Parameter(ValueFromPipeline=$true)]
            [string[]]
            $As = ('test1','test2','test3','test4')
        )
    
        Begin
        {
            [string[]]$b = $null
    
            function go ()
            {
                $b += $A 
            }
    
        }
        Process
        {
            Foreach ($A in $As) 
            {
                go
                Write-Host $b  #debug line
            }
        }
        End
        {
            # Write-Host $b  #commented out due to unknown error
        }
    }
    

    If I paste this in my shell to run/load it into memory and then execute the test-begin cmdlet, I would expect to get the four values (first one, then the first repeated with the second, and so on until all are listed). If the thing was working I would just write to host once at the end, but I am debugging currently, see comments.

    I know the issue has something to do with defining the function in the begin block. I can't find the answer on my own.

    #34281
    Profile photo of Don Jones
    Don Jones
    Keymaster

    So... Apologies if this is a stupid question; you haven't defined any parameters as accepting pipeline input, right? So the begin and process blocks are effectively ignored. Those only work in pipeline input mode. Your code is running straight through, with all values in the parameter as an array.

    So I guess, what was the expectation?

    #34282
    Profile photo of Tony Pagliaro
    Tony Pagliaro
    Participant

    Sorry I dumbed it down a little too much. The param block now looks like the param block in my bigger script (after edit). I am expecting to get some output (whether piped in or not).

    ps> $a = ('test1','test2','test3','test4')
    ps> $a
    test1
    test2
    test3
    test4
    
    ps> $a |test-begin
    
    
    
    
    
    ps> 
    
    #34295
    Profile photo of Max Kozlov
    Max Kozlov
    Participant

    Variable visibility scope hides somewhere around 🙂

    [CmdletBinding()]
    Param
        (
            [Parameter(ValueFromPipeline=$true)]
            [string[]]
            $As = ('test1','test2','test3','test4')
        )
    BEGIN {
      $s = @()
      function go {
        $s1 = Get-Variable -Scope 1 -Name s
        $s1.Value += $a
      }
    }
    PROCESS {
      foreach ($a in $as) {
        go
      }
    }
    END {
      $s
    }
    
    #34305
    Profile photo of Don Jones
    Don Jones
    Keymaster

    yeah, in your original function, $b is private to go(), and doesn't exist after go() finishes running. You technically should be returning $b as the result of go(), not merely relying on the variable's existence. Similarly, go() shouldn't be relying on $A, either. $A should be passed into go().

    function test-begin
    {
        [CmdletBinding()]
        Param
        (
            [Parameter(ValueFromPipeline=$true)]
            [string[]]
            $As = ('test1','test2','test3','test4')
        )
    
        Begin
        {
            [string[]]$b = $null
    
            function go ($in)
            {
                $b += $in
                write $b
            }
    
        }
        Process
        {
            Foreach ($A in $As) 
            {
                $out = go $A
                Write-Host $out  #debug line
            }
        }
    }
    

    Functions shouldn't rely on anything external to themselves, and nothing should rely on anything internal to a function. Parameters provide functions their input, and the pipeline conveys their output to their caller.

    Note that my example here is still not going to do what you want; $b gets a fresh scope overtime go() runs. So it's effectively the same as "$b = $in". I'm not sure what the goal was, but variable scope is what's confusing you.

    #34345
    Profile photo of Tony Pagliaro
    Tony Pagliaro
    Participant

    I'm not sure why it's so hard to get a function to work with both pipeline and argument input. There are tons of PS cmdlets that do this. But since this function is only PART of my overall script, I can choose one avenue and go with it. Whichever works.

    Max, Yes the variable scope. I need to learn more "about_" that.

    However the output is one string and not an array:

    ps> $c
    test1
    test2
    test3
    test4
    ps> test-begin -As $c
    test1 test2 test3 test4
    ps> $c | test-begin
    test1 test2 test3 test4  
    

    If I run the script in parts it works OK

    ps> [string[]]$b = $null
    ps>  function go ()
    >>         {
    >>             $b1 = Get-Variable -Scope 1 -Name b
    >>             $b1.Value += $A
    >>         }
    >>
    ps>  Foreach ($A in $As)
    >>         {
    >>             go
    >>         }
    >>
    ps> $b
    test1
    test2
    test3
    test4
    ps>
    

    Here's the function I used above:

    function test-begin
    {
        [CmdletBinding()]
        [OutputType([string[]])]
        Param
        (
            [Parameter(Mandatory=$true,
                       ValueFromPipeline=$true,
                       Position=0)]
            [string[]]
            $As
        )
        Begin
        {
            [string[]]$b = $null
    
            function go ()
            {
                $b1 = Get-Variable -Scope 1 -Name b
                $b1.Value += $A 
            }
        }
        Process
        {
            Foreach ($A in $As) 
            {   go   }
        }
        End
        {   Write-Host $b   }
    }
    

    For some background, my full script gets fed a list of hostnames. It pings them and if alive, it checks for a 3rd-party service and other things. If the service is missing it deploys it using psexec. The part of my script we're having trouble with in this thread tallys the hosts that get deployed so it can report results when complete. The embedded function being defined is the 'deploy' function and the last line of this function is supposed to add the current hostname to a variable array for reporting. But for our purposes here, my dumbed-down version produces the same issue. The main function gets an array of strings, the nested function adds the current string to a new variable for output.
    Don, your method looks cleaner, but I couldn't get it to work (after removing the debug lines, and putting the write $b at the end, there is no output). Maybe after reading the above you could recommend some other way to achieve my goal of reporting deployed hosts.
    Thanks to both of you!

    #34350
    Profile photo of Don Jones
    Don Jones
    Keymaster

    Well, to be fair, I've never had any problem getting a function to run equally well in either mode. And I don't think that's your problem here, either. I think your problem has a lot more to do with using nested functions (which get awkward), and not tracking variable scope (which nested functions make harder to do). This business of using out-of-scope variables to pass data round is a horrible idea, and it trips people up all the time. It's a bad design, and I truthfully think that's where some of your problems stem from.

    Take this:

            {
                $b1 = Get-Variable -Scope 1 -Name b
                $b1.Value += $A 
            }
    

    PowerShell passes variables ByValue, not ByReference. $b1 does not "point" to the original higher-scope $b; it is a copy of $b. Changing $b1 doesn't change $b. Further, you're still referring to out-of-scope variable $A. Not trying to be a jerk here, but you're working against the way the shell prefers to work, and you're going to make life harder on yourself if you go that route.

    If I had a function whose sole purpose was to append a value to a variable, I wouldn't write that as a function. I realize that you're just doing that by way of illustration, but let's look at what you're trying to do:

    "The embedded function being defined is the 'deploy' function and the last line of this function is supposed to add the current hostname to a variable array for reporting."

    Here's how I'd handle that.

    param([string[]]$hostnames)
    $success_hosts = @()
    $failed_hosts = @()
    
    function check_whatever([string]$hostname) {
     # check stuff.
     if ($stuff -eq $working) { write $true } else { write $false }
    }
    
    foreach ($hostname in $hostnames) {
     if (check_whatever $hostname) {
      $success_hosts += $hostname
     } else {
      $failed_hosts += $hostname
     }
    }
    

    In other words, I'm keeping all the data in the scope where it needs to be used. I'm not asking the embedded check_whatever function to look "up" the scope to see what the current host is – I'm explicitly passing in the host to check. And check_whatever isn't concatenating values to a variable it doesn't "own." It's returning a True/False value, and the parent scope – which is the one tracking success/fail – does the concatenation.

    I wrote it before, but I'll emphasize it: Functions should not access, or modify, anything outside their own scope, ever. Pass in what they need as parameters. They do their job, and produce a result to the pipeline. The parent can then examine that result, and do whatever needs to be done as a consequence. If you're ever in a situation where you think, "well, this is an exception to the rule – in this case, I need to access out-of-scope stuff," then you're not thinking about it the right way.

    All this is triply true for a function running in the pipeline, because the scope has a different lifetime. The BEGIN{} block runs before pipeline input has been bound, so you don't necessarily have all your variables. The PROCESS{} block runs, but only one value will have been bound from the pipeline. This extra scope complexity can really throw a wrench in things when you're not following recommended practices, and you end up working around the way the shell was designed.

    I think you'll find things easier if you kind of re-examine your overall approach to tracking data within the process. I'd be happy to help pseudo-code it with you, if you want, to maybe offer a different perspective or some alternate approaches.

    #34357
    Profile photo of Max Kozlov
    Max Kozlov
    Participant

    I try to stay away from coding style and other things, Don said about but .....

    STOP using Write-Host!

    I try your code and it work as intended, but in the end{} block you use write-host, that out any arrays as string. and out it to host, thus you can't use your function like $var = test-begin xxx,yyy,zzz

    compare
    $a = 1,2,3,4,5
    $a
    Write-Host $a

    #34401
    Profile photo of Tony Pagliaro
    Tony Pagliaro
    Participant

    Max, thanks for solving that mystery.

    Don, Thank you for the detailed reply. I think you're right (as always). I don't think you're a jerk. I appreciate a more efficient way to do this. I'll sit down again when I have a chance to review this and I'll let you know if I need more help. Thanks!

Viewing 9 posts - 1 through 9 (of 9 total)

You must be logged in to reply to this topic.