Please add new line.

Done.

Please correct indentation.
filePath can be null here and raise exception.
Original cmdlet wrap this:

if (filePath)
{
   FileInfo file = new FileInfo(filePath);
   WriteObject(file); 
}

Thanks, I wasn't sure what condition could fall into that, that would not trip the catch return. Thanks for clarifying.

@PowerShellTeam Is it correct? What is a policy about copyright?

First cmdlet I've writte and I saw that at the top of every other file, wasn't really sure what the protocol was. It's probably wrong. Removing now.

@kittholland :-) My question was not addressed to you. Please don't rush to remove.

Repeating code. Please use a variable.

Removed duplicate GetEnvironmentVariable call. Thanks

We usually use a space after //.
And the comment is obvious and likely unnecessary.

@lzybkr I found some powershell built in cmdlets put the main logic in BeginProcessing when the logic is simple, such as New-Guid, New-TemporaryFile, and Write-Information before @iSazonov adding pipeline support to it. Why did we choose to put main logic in BeginProcess instead of EndProcess for simple cmdlets like this one? I don't see any obvious performance difference.

Putting the logic of a simple cmdlet in the end block does protect against stupid pipelines, e.g.

function f { begin { write-host 'in begin f'}}
function g { end { write-host 'in end g'}}
g | f

This will print

in begin f
in end g

which is a little weird that the logic for a downstream cmdlet finishes before the upstream command (note no objects are being piped).

In others, you could more easily write some obfuscated (as in, hard to figure out the real logic) pipelines if you use begin instead of end.

Thanks @lzybkr, this is a great example. So it looks the guideline should be using EndProcessing (or End block in script) for simple logic that doesn't require pipeline input.

I think the reason we have those cmdlets with main logic in Begin/BeginProcessing is that the pattern was copied from other existing cmdlets, without knowing why it should be put in Begin/BeginProcessing.

@kittholland could you please move the main logic to EndProcessing method given the discussion here? Thanks!

Yes I will move it.

I left as is because this is an existing cmdlet and did not want to potentially alter any existing implementations (and I did not feel that I could fully comprehend what those possible scenarios would have been).

Yep, totally understood. Thanks!

This implies that we need to open a new Issue with analysing what cmdlets must be corrected.

Revert this change please.

Revert this change please.

Revert this change please.