Now that the PowerShell Scripting Games for 2013 are well underway, I thought I'd share my thoughts and impressions on what I've seen. I'm very impressed with the number of entries and generally the quality is pretty good. But as a judge I see repeated items that bear comment. These comments are in no particular order of importance and in some cases are really a matter of personal preference.
ManageEngine ADManager Plus - Download Free Trial
Exclusive offer on ADManager Plus for US and UK regions. Claim now!
If you are going to use Write-Host (and want to save puppies) to display informational or progress messages, please use one of the color parameters so that your message can be differentiated from your object output. Ideally for advanced events you should be using Write-Progress. Not sure we've had events that call for Write-Progress but keep it in mind.
I generally dislike entries that are clearly overwrought and over-thought. Sometimes the problem can be solved simply. Don't feel you have to use every trick in the PowerShell play book in order to score points. I'm a big proponent of the right tool for the job. On a related note, if you can solve the challenge with a PowerShell one-liner, don't feel you need to write it as a long single line command. Take advantage of PowerShell parsing. This is very hard to read:
Get-WmiObject win32_operatingsystem -comp $computers | select PSComputername,@{Name="TotalMemoryMB";Expression={[int]($_.TotalVisibleMemorySize/1KB)}},@{Name="FreeMemoryMB";Expression={[math]::Round($_.FreePhysicalmemory/1KB,2)}},@{Name="PercentMemoryFree";Expression={[math]::Round(($_.freephysicalmemory/$_.totalvisibleMemorySize)*100,2)}} | Sort PSComputername | Export-CSV -Path c:\work\osreport.csv -Encoding ASCII -NoTypeInformation
I'd much rather see a one-liner formatted like this:
Get-WmiObject win32_operatingsystem -comp $computers | select PSComputername, @{Name="TotalMemoryMB";Expression={[int]($_.TotalVisibleMemorySize/1KB)}}, @{Name="FreeMemoryMB";Expression={[math]::Round($_.FreePhysicalmemory/1KB,2)}}, @{Name="PercentMemoryFree"; Expression={[math]::Round(($_.freephysicalmemory/$_.totalvisibleMemorySize)*100,2)}} | Sort PSComputername | Export-CSV -Path c:\work\osreport.csv -Encoding ASCII -NoTypeInformation
In full scripts, I'd like to see more use of #Requires -version X so that I can tell what features you might be using. Related to that, in advanced scripts if you are using a parameter attribute like Mandatory or ValueFromPipeline, v3 scripts should use them like this:
[Parameter(Position=0,ValueFromPipeline,Mandatory)]
You don't have to explicitly state that Mandatory is equal to True. Although in v2 you would need to do this:
[Parameter(Position=0,ValueFromPipeline=$True,Mandatory=$True)]
Which brings me to another minor irk, an object that has a boolean value doesn't need a comparison operator. The whole point of something like an IF statement is to evaluate if the expression in the parentheses is true or not. If the object is already a boolean, there's no need.
$found = $True #bad form if ($found -eq $True) { #deleting computer from active directory #... } #better form if ($found) { #deleting computer from active directory #... }
I'm also not a big fan of creating custom objects with lots of Add-Member commands. I think this makes the code harder to read and doesn't really buy you much. I think using a hashtable with New-Object is much easier to read and just as effective. Plus in v3 we can now have ordered hashtables and even use [pscustomobject].
$os = Get-CimInstance -class win32_operatingsystem -ComputerName $computer $computersystem = Get-CimInstance -ClassName win32_computersystem -ComputerName $computer $is64Bit = If ($os.OSArchitecture -match "64") { $True } Else {$False} $Inventory = [ordered]@{ Computername = $os.PSComputername OperatingSystem = $os.Caption Installed = $os.InstallDate Model = $computersystem.Model Mfg = $computersystem.Manufacturer Is64Bit = $Is64Bit } [pscustomobject]$Inventory
Finally, be very careful of including formatting commands in your entries, unless the event specifically calls for it. This is especially true if you are writing a function. When you include formatting directives at the end of the function, it can't be used anywhere else in a PowerShell expression.
Don't get me wrong, there is a lot of good PowerShell which I'm happy to see:
- Using Join-Path to build paths
- Using Test-Path to validate
- Plenty of internal comments
- Using Test-Connection to verify computers are online
- Meaningful variable names
So keep up the good work and on to the next event!