PoshCode / PowerShellPracticeAndStyle

The Unofficial PowerShell Best Practices and Style Guide
https://poshcode.gitbooks.io/powershell-practice-and-style
Other
2.24k stars 289 forks source link

Begin/Process/End or Process only. No Process/End, Begin/End, Begin/Process. #181

Closed Null-Fault closed 1 year ago

Null-Fault commented 1 year ago

https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_functions_advanced_methods?view=powershell-7.3

Pretty straight forward. Microsoft states:

Using either a begin or end block requires that you define all three blocks. When using any block, all PowerShell code must be inside one of the blocks.

The examples in the style guide do not fit Microsoft's recommendation.

indented-automation commented 1 year ago

The documentation is incorrect and that should be fixed, not this guide.

The documentation also contradicts itself a bit on this point, which to my mind reinforces the need to correct the documentation. It includes the following assertion:

You can use a `process` block without defining the other blocks.

Raised PR: https://github.com/MicrosoftDocs/PowerShell-Docs/pull/10521

Null-Fault commented 1 year ago

The documentation may be incorrect about what is technically possible, but that does not mean that is not stylistically ideal.

Why would you only have an end block? Would it not be better to just have no begin/process/end blocks in that case?

Why would you only have a begin block? Would it not be better to just have no begin/process/end blocks in that case?

Supposedly you need a process block to iterate pipeline objects (happy to be wrong there) and that is the minimum of what is needed, but you don't have to define begin/end blocks to use a function nor do they add meaningful clarity if used by themselves. There are many things that are technically possible but not stylistically ideal.

indented-automation commented 1 year ago

I don't really understand your point. But I do want to be clear at the outset I'm treating this as a problem of two halves:

  1. Any change to the style guide when there's a good basis or argument for making that change.
  2. A technical requirement postulated by the about_ docs.

1. Changes to suggested style

I strongly suggest you highlight the part of this guide you think should change, and comment based on that. You cite MS docs, but not really how it applies to the guide, or really what you want changed in this guide.

Is your comment directed at this statement?

https://github.com/PoshCode/PowerShellPracticeAndStyle/blob/master/Style-Guide/Code-Layout-and-Formatting.md#prefer-param--begin-process-end

It is fine to disagree with the style guide and to propose change. Style is inherently opinionated. We (or others) may end up not agreeing, but that's the nature of opinion and that outcome is fine.

Why would you only have an end block? Would it not be better to just have no begin/process/end blocks in that case?

I regard this as fair comment. I would, personally, not have an end block if I had no others. I'd regard it as clutter.

Why would you only have a begin block? Would it not be better to just have no begin/process/end blocks in that case?

I have no use case for this, but they are not technically equivalent. end is not a substitute for begin when operating in a pipeline.

I cannot imagine a use-case for this, but I would not seek to restrict based on my lack of imagination. But I cannot find any part of the guide that asserts use of end in place of begin. I possibly did not look hard enough.

Supposedly you need a process block to iterate pipeline objects (happy to be wrong there)

You can technically do it all in end via $input, bit magic though.

Are you suggesting that when I have process, I should maintain an empty begin and end block? I'm not clear on this point.

2. The about_ document

About documents are technical documents, not style guides. The highlighted assertion, which I have corrected by the (merged) PR linked above, is:

Using either a begin or end block requires that you define all three blocks. When using any block, all PowerShell code must be inside one of the blocks.

This is framed as a technical requirement ("requires that you"), not a style choice.

It tells me that if I want to do this:

function Write-Number {
    param (
        [Parameter(ValueFromPipeline)]
        [int]
        $Number
    )

    process {
        Write-Host $Number
    }
}

That I would in fact have to do this:

function Write-Number {
    param (
        [Parameter(ValueFromPipeline)]
        [int]
        $Number
    )

    begin {}

    process {
        Write-Host $Number
    }

    end {}
}

Implying my first block would not work because I had omitted the two blocks I wasn't using.

Since this asserted technical requirement is incorrect I raised a (now merged) fix for that.

Null-Fault commented 1 year ago

Let's refer back to the document a few times then I'll go with some examples.

If you do not use any blocks then everything is automatically put into an end block. This is stated in the document:

You aren't required to use any of these blocks in your functions. If you don't use a named block, then PowerShell puts the code in the end block of the function.

So we know we do not need to use an end block if we do not need to use other blocks.

You can use a process block without defining the other blocks.

The process block is required for processing objects via the pipeline. It does not require begin/end blocks.

What does this mean? This means that this is redundant for a regular function:

function Write-Number {
    param (
        [int]$Number
    )
    end {
        Write-Host $Number
    }
}

And this is more straightforward for a regular function:

function Write-Number {
    param (
        [int]$Number
    )
    Write-Host $Number
}

You would also never do this:

function Write-Number {
    param (
        [int]$Number
    )
    begin {
        Write-Host $Number
    }
}

For processing items via the pipeline, your first example is ideal for the minimum needed for a function for pipeline input:

function Write-Number {
    param (
        [Parameter(ValueFromPipeline)]
        [int]$Number
    )
    process {
        Write-Host $Number
    }
}

However if you're going to use begin/end blocks then you would likely be setting something up to either return or tear down later like:

function Write-Number {
    param (
        [Parameter(ValueFromPipeline)]
        [int]$Number
    )
    begin {
        $Numbers  = @()
    process {
        $Numbers += $Number
    }
    end {
        return $Numbers
    }
}

The order of preference for the blocks should be: No blocks Only process block All blocks

The last one is the one most easily debatable. I hope I have shown why you usually would not want to use begin/end blocks and if you do then they should be used with process, but process can be used all by itself. It usually should be.

indented-automation commented 1 year ago

You need to associate the comments above with the parts of the guide that you'd like to see changed.

As I said above, it's fine to suggest change where you have a good case for that. However, someone needs to write that change into the guide. Having an idea of what specifically should change is critical. You can, of course, raise a pull request of your own to suggest change.

I personally would disagree with the order of preference as written (the final "All blocks"). For example, I might contrive a function whose only job is to add a value to the end of a pipeline:

function Write-Number {
    param (
        [Parameter(ValueFromPipeline)]
        [int]$Number
    )
    process {
        $Number
    }
    end {
        9999
    }
}

I don't wish to pretend this is a good function, but I don't need begin in this scenario, I therefore don't want to include it under an "All blocks" directive.

Similarly, I might want to include begin and process, but have no use for end. I have a few of these where begin does a bit of advanced parameter validation or stages something process might use.

My own guidance would therefore be:

  1. Prefer implicit use of the default block when no other named block is required.
    • end for function.
    • process for filter.
  2. Use named blocks to support a pipeline as appropriate for the command.
  3. Do not include empty named blocks.

I have custom script analyzer rules that check for use of process when an input pipeline is not in use (for example) to enforce some of these patterns.

That's my own though, it is how I write my code now. My own preference does not necessarily need to line up with the style guide and does necessarily affect suggestions you might make.