Skip to content

Consider revising PERF-02 #148

Open
Open
@jdtrouble

Description

@jdtrouble

I have some suggestions for improving "PERF-02 Consider trade-offs between performance and readability". I hope it's OK to group multiple "issues" here rather than spamming the Issues section.

The first issue, the page directs a user to drop into using a .NET framework to do something (basically read a file) when a PowerShell cmdlet already exists to do the same thing. The page actually called out the appropriate cmdlet a moment before. This is in contradiction to other advice that, generally, you should avoid using .NET or external tool when PowerShell can effectively do the same thing.

Second, the page uses the .NET code to do something that PowerShell handles natively with the Pipeline. I use Powershell a lot in my professional career, and I can say that 80% of time I'm handling collections of objects. Which computers in this list have x.yy version of Audio Driver? Which AD users are locked and/or password expired? The pipeline is king, the king is dead, all hail the king.

Get-Content -Path file.txt | ForEach-Object -Process { <#...#> }
Of the options in the article, this is absolutely the best way to approach the input.

Heck, even "one object" collections circumstantially benefit from going through the pipeline. Consider the scenario, I want widget.exe to not run. For the purpose of this example I expect at most one instance of wdiget.exe to run. And maybe it's not running presently, I don't care. But I certainly want to stop, if it is running.
Stop-Process -Name widget
This will generate an error if widget.exe is not running. I don't want an error message in this case. If I'm stopping on errors, this will break the script.

Get-Process -Name widget | Stop-Process
Even if there is never 2 or more instances of widget.exe, this is the best way to handle the use case (and IMO, it looks great). If the process is not running, then nothing gets passed to Stop-Process. Rather than generating an error, Stop-Process just sits there and twiddles its thumbs.

Less important but worth mentioning, the page uses what Wikipedia would call "Weasel words." See:
https://en.wikipedia.org/wiki/Wikipedia:Manual_of_Style/Words_to_watch#Unsupported_attributions
Examples, "many folks in the community", "Most folks", "Some would argue". Who are these people, and why should I care? I was sysadmin at a factory and I love the assembly line approach to my control scripts. I often script in this format:

param (
    # Bunch of variables I might want to customize in the future
)

# in production, functions actually reside in a module psm1 file I wrote, which I import here
function Verb-Stuff {
    param ( 
        [parameter(Mandatory,ValueFromPipeline)][type]$Object
        # Other parameters ...
    )
    Process {
        try {
            # Do stuff ...
        } 
        catch {
            # error handling
        }
    }
}

Get-Stuff -Property $Parameter | Where-Object { <#filter stuff#> } | Verb-Stuff | Out-Stuff 

I find the above to be very aesthetically pleasing, and easy to follow. I understand that not everyone will agree, but honestly I don't care. Aesthetics are nice when talking about tab lengths, max line lengths, NamingConventions, etc. When you are writing code in production, "aesthetics" are not a good reason to change the function of the code. To do so risks unplanned problems.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions