I've been making headway in reviewing and judging entries in the 2011 Scripting Games. I know there has been a lot of discussion about the lack of comments and I'm doing what I can with entries I judge, but I'm not guaranteeing anything. What I will do is put down some overall comments and impressions that I hope you can use for the remaining events.
ManageEngine ADManager Plus - Download Free Trial
Exclusive offer on ADManager Plus for US and UK regions. Claim now!
Overall, I must say the caliber of PowerShell is much improved over years past. I don't see as many scripts that are PowerShell versions of VBScript. People are really getting the PowerShell paradigm. One thing that might still be eluding some is that writing code that you would use in an interactive session and something you want to re-use in a script are two different animals. When using PowerShell in an interactive session, the goal is efficiency: getting the job done with the least amount of effort and manually handling problems or errors as they arise. When writing a script you are writing PowerShell that can be re-used by yourself and others. You should only have to write it once.
All of that said, here is what I'm looking for in an entry.
For Everyone
Avoid the use of aliases and use full cmdlet names. I can make an exception for well-known aliases like DIR. But does everyone looking at your code really know what % or iex refer to? I would like to see this concept extended to parameters as well. Use full parameter names, even if they are positional. The goal is clarity. There is no performance penalty for the number of characters in a PowerShell expression.
There should be plenty of comments throughout the script that explain what the script is doing. You don't need to write War and Peace but I should be able to tell what a section of code will do. Comments are also critical if you script has several scripts and is jumping around. Otherwise it is hard to follow the flow. But don't write comments just to have a comment.
Variable names should be meaningful. Scrolling down in a long script I may forget what $c is, but $computername I can figure out. While on this subject, I always recommend using names that are used throughout PowerShell. Thus, use $computername, not $server or $sys or whatever. Write your code so that it is consistent with the rest of PowerShell. It makes it easier to follow and integrate.
I'd like to see at least basic error handling. For beginners, take advantage of some of the Test cmdlets and use an If construct. By the way, when testing a Boolean you don't need to use a comparison operator.
$b=$True" if ($b -eq "True") { #do something }
Because constructs lke IF and WHERE are designed to evaluate expressions looking for True or False, all you need to do is express the variable.
$b=$True" if ($b) { #do something } #or if you want to check the inverse if (-NOT $b) { #do something }
The last thing that I really look for is if the script works with objects. Using Write-Host to display some information is OK I suppose for the most simplest of tasks but it is limiting. Because Write-Host doesn't send data to the pipeline you can't format, export, sort, group, convert or do anything else with it. I like Write-Host for progress messages, especially when using a foreground or background color so I can differentiate messages from output.
Related to the quest for objects is the continued use I see of the RETURN keyword. In PowerShell this is potentially problematic. Don't think about returning a value, thing writing objects to the pipeline. In case you didn't know, when PowerShell encounters RETURN, it writes "value" to the pipeline and stops. If there is more data or more to your script it never executes. Here's an example.
$Processes=Get-Process ForEach ($process in $Processes) { $isPrivateBuild = (Get-Process -ID $process.id -FileVersionInfo).IsPrivateBuild $obj=New-Object -TypeName PSobject -Property @{ ID = $process.ID Name=$process.ProcessName Computername=$process.machinename PrivateBuild=$isPrivateBuild } Return $obj }
This will only return a single object. Instead do something like this:
$Processes=Get-Process ForEach ($process in $Processes) { $isPrivateBuild = (Get-Process -ID $process.id -FileVersionInfo).IsPrivateBuild $obj=New-Object -TypeName PSobject -Property @{ ID = $process.ID Name=$process.ProcessName Computername=$process.machinename PrivateBuild=$isPrivateBuild } Write-Output $obj }
Beginners
For beginners another common problem I see is using Get-WMIObject to get the local computername. Instead use $env:comptername to retrieve the %COMPUTERNAME% environmental variable.
When breaking lines you don't need to use the backtick after anything that PowerShell knows needs to be completed. If you have a long line you want to break, just press return after an opening {, ( or |. You should only need the backtick when you are breaking a line in a spot that PowerShell isn't expecting.
Finally, look at your script. How much rewriting would it take for someone else to run it? How flexible or re-usable is it? The more that you can meet that need the more extra points you'll likely score.
Advanced
For advanced events I think you should have good comment based help, should be using advanced parameter attributes and complex error handling in addition to everything I've mentioned above. I also look for more modularity and the use of functions. Even more so with this group I look for code that is flexible and resilient. I shouldn't have to go back and modify hard coded values to get your script to work.
However, even though I look for a lot from an advanced entry there is a fine line between complexity for the sake of complexity and code or comments that contribute clarity and value. For me, a script that will warrant a 4 or 5 will have a certain degree of elegance, which I know is a subjective element but you know it when you see it. And I certainly don't claim that everything I write and publish is elegant by any means. For me, elegance means that the script leverages PowerShell's strengths with the just the write amount of all the elements we look for in a script, that gets the job done. I suppose elegance goes hand in hand with efficiency and I don't mean constructing a one line expression. I've seen some scripts that for lack of a better term feel heavy handed. Yes, they may get the job done but it feels like a kitchen sink approach: let me throw in all the techniques I know. Even with comments these scripts are hard to decipher and I wouldn't look forward to debugging one.
In summary I encourage you to address these concepts in your PowerShell scripting:
1. Functionality: Does it work?
2. Objects: What are you writing to the pipeline?
3. Clarity: Are your using full cmdlet and parameter names? Are there comments? What are your variable names? Can I "read" your script?
4. Re-Use: Are you hard coding values? How flexible is your code?
5. Elegance: This is almost a Zen-like element. Is there a balance between what is written and what is really required?
I look forward to the rest of the games and wish everyone the best of luck.
I hope if goes without saying, but just to be safe, these are my comments only and do not reflect the opinions of any other judge or The Scripting Guys. These are suggestions based on my experiences as a scripter, author and trainer.
Thanks Jeff for the comments. This is my first time in the games and I must say that I have already learnt a lot from the first week. From simple things like the computername environment variable to how to correctly format parameter sections and comment based help. I have a great sense of achievement from only picking up powershell scripting in the last year (I never could learn vbscipt). Looking forward to the rest of the challenges.
Thanks Jeff, Wish I had seen the post before completing the games! As a new comer to PS I have used many of the no no’s you discussed. Thanks for explaining why not to do that! I really liked the Return example!