Don Jones
Contents
Do Not Overuse Variables
Meaningful Variable Names
Simplify Comparisons
Don't Break Your Script
Loop the Loop
Don't Be So Silent
Show Me Your Scripts
I recently made an offer (which still stands) to readers of my ConcentratedTech.com blog: Send me your Windows PowerShell scripts and I'll review them and offer any suggestions I have to help improve them. The idea actually came from a survey my co-bloggers and I ran—a reader suggested that this would provide a great, real-world way for people to improve their scripting skills. Over the past few months, I've noticed a few trends as I find myself making several particular suggestions repeatedly. So I've compiled a list of them to share with you here.
Let me first state, however, that any script that gets the job done (successfully and without collateral damage, of course) is a good script. Some of the things I'm suggesting may seem nitpicky and some of them certainly are. However, each of my suggestions does have some distinct advantages, which I'll try to point out as I go.
Don't take offense if you're actually doing some of the "bad practices" I point out. I know that a lot of people that script in Windows PowerShell started out scripting with VBScript and some of these "bad practices" are actually very common in the VBScript world. This is a great opportunity, then, to learn how Windows PowerShell and VBScript are a bit different and how to write scripts that make use of the unique advantages Windows PowerShell has to offer.
With that said, here are my top six common suggestions I've made over the past few months.
Do Not Overuse Variables
Sure, variables are useful, but let's not get carried away. I've been seeing a lot of this kind of scripting:
$varfile = "c:\names.txt"
$varexists = Test-Path $varfile
if ($varexists –eq $false) {
Write-Output "The file $varfile does not exist"
break
}
$varnames = Get-Content $varfile
There are a few practices I'd like to see changed in an example like this. First and foremost, there's little need to stick all that information into variables. It's possible to rewrite this with fewer variables. Why bother? Well, the shell might not care how many variables you use, but I'm a big, big fan of making scripts as readable as possible so someone can read your script on paper or in a script editor and be able to figure out what the script is doing. The more variables you include, the more someone will have to track in his head—and the less likely he'll be able to figure out what the script is up to. If I can't figure out the script on paper, I'm less likely to be able to debug the script if there's a problem. So I'd start by rewriting this sample as follows:
$varfile = "c:\names.txt"
If ((Test-Path $varfile) –eq $false) {
Write-Output "The file $varfile does not exist"
break
}
$varnames = Get-Content $varfile
All I'm really doing is eliminating the $varexists variable and directly utilizing the output of Test-Path. This way, I don't have to look at the If construct, see $varexists, and then go back and figure out what $varexists is supposed to contain. It's less for me to keep track of in my head. Notice that I left $varfile; this is because I use that information three times—if you're re-using information multiple times, then it's worth putting into a variable.
Meaningful Variable Names
Continuing with the previous example, always remember that variable names should help you remember what's inside a variable. You don't need to use name prefixes like "var" because the $ character tells you it's a variable. In fact, name prefixes like "str" for strings or "int" for integers are outdated ("that's so 1993" is what I say when I'm teaching classes). Just use straightforward variable names. Here's the example rewritten:
$filename = "c:\names.txt"
If ((Test-Path $filename) –eq $false) {
Write-Output "The file $filename does not exist"
break
}
$computernames = Get-Content $filename
Simplify Comparisons
I'm not a fan of comparison operators in Windows PowerShell—ones like –gt, -lt, -eq, and so forth. My lack of fondness probably comes from my long experience with VBScript, where I could use familiar operators such as >, <, and =. Many of you probably recognize these operators from grade school math lessons, and so they require less mental parsing when you read them. Whenever possible, then, I like to avoid using an operator. And any condition where you check for something to be True or False is an opportunity to get rid of an operator:
$filename = "c:\names.txt"
If (-not (Test-Path $filename)) {
Write-Output "The file $filename does not exist"
break
}
$computernames = Get-Content $filename
Because the Test-Path cmdlet returns True or False, there's no need to compare it to True or False. Everything inside the If constructs parentheses needs to boil down to True or False. Normally, you would use a comparison to achieve that, but if whatever is in there is already generating True or False, you can just use that info directly.
I should note that the –not operator is one I actually do like. It's more like common English than the equivalent != operator used by many languages.
Also, I should comment on the parentheses I'm putting around Test-Path and its input parameter, $filename. I've started enclosing commands in parentheses because it helps me visually grasp them as a single unit. Windows PowerShell uses spaces, rather than some other character, to separate things like commands, parameter names, and parameter values. Enclosing the entire command string in parentheses helps me see it all together, without forcing me to actually look at the spaces and determine where the command starts and stops.
Don't Break Your Script
The last suggestion I have for this running example is the way its logic is built. If the file doesn't exist, I break out of the script. If the file does exist, I never execute the Break statement and so the script continues. Whenever possible, I like to avoid using Break to exit a script. I'd rewrite the example—again—as follows:
$filename = "c:\names.txt"
If (Test-Path $filename) {
$computernames = Get-Content $filename
# do whatever we planned to do
} else {
Write-Output "The file $filename does not exist"
}
This way, I've simplified the logic in the If construct. I've also eliminated the need to use the Break statement, which reduces confusion about what the script is doing. I'm the first to admit that this suggestion is absolutely a nitpick, but I think good coding practices help make better scripts, and this is definitely a better practice than bailing out of a script halfway through. This type of construct is easier to debug, and it's much easier to follow on paper if you're looking at a script and trying to figure out what it does.
Loop the Loop
Several of the scripts I reviewed needed to read information from a text file and loop through each line of the file. For example, you might want to contact a series of computers, so you could list their names, one name per line, in a text file. Here's a common construct I've seen:
$line = 0
$names = get-content c:\names.txt
while ($line –lt $names.length) {
# do something with $names[$line]
$line++
}
This loads the file into the variable $names and then manually enumerates through the collection as if it were an array. The $line variable keeps track of which line I'm working with. It works fine—but I'm doing a bit more work than I need to do, and I'm making the code a bit more difficult for someone else to read later. The ForEach construct in Windows PowerShell is a bit more appropriate for enumerating collections. (Technically, Get-Content returns a collection, not an array). Try this instead:
Foreach ($name in (Get-Content c:\names.txt))
{
# do something with $name
}
There are a few advantages here. In keeping with my previous suggestion, I'm putting the call to Get-Content right within the construct, since I think that makes it a bit clearer as to what's going on. ForEach will enumerate through the entire collection, regardless of how long it is. Each time through, it will take the next item in the collection and put that item in the $name variable. So the variable name represents what it contains—a computer name. And that makes the script easier to read and to follow.
Windows PowerShell Q & A
Q Is there a way to define a permanent custom alias in Windows PowerShell?
A If you've ever created a new alias, you've probably noticed that it goes away when you close the shell. For example, this creates a new alias d, which is a shortcut to the Get-ChildItem command:
New-Alias d Get-ChildItem
But when I close the shell, the alias is lost. The trick is to create a Windows PowerShell profile. In your Documents folder, create a new folder called WindowsPowerShell. Within that folder, create a text file named profile.ps1. In that text file, place any commands that you want to execute automatically each time the shell launches—including a new alias definition. The profile ensures that the commands run every time you launch the shell, creating a "permanent" alias that's always available in Windows PowerShell.
I don't need to worry about what line I'm working with or keep a $line variable to keep track of that—the ForEach construct does that for me. Eliminating three lines of code makes the code a bit more succinct and requires you to keep track of fewer things in your head when you're trying to figure out what the script does.
Don't Be So Silent
The one practice I really want to nip in the bud is the practice of suppressing shell errors in an entire script. Say you're looking at a script that includes this line right at the top:
$ErrorActionPreference = "SilentlyContinue"
Stop it immediately.
This doesn't make errors go away, but it does suppress them. Yes, there are certainly times when you will anticipate and want to disregard a certain error, such as when you're using Windows Management Instrumentation (WMI) to connect to remote machines that are potentially unavailable. But there's never a reason to globally disregard errors. Error messages are useful and point out problems that you can sometimes fix.
If you want to disregard errors for a particular cmdlet, you can do so by adding the –EA "SilentlyContinue" parameter to that cmdlet. If you want to disregard errors for several commands in a row, either add the –EA parameter to each or add the $ErrorActionPreference line immediately before those commands and after those commands, add:
$ErrorActionPreference = "Continue"
This sets error handling back to normal.
Personally, I don't like turning off errors at all unless I'm handling those errors myself. For example:
Trap {
# output error to a log file or something
}
Get-WmiObject Win32_Service –computerName $remote –EA Stop
This brief example shows how the –EA "Stop" parameter will generate true exceptions, rather than just error messages, giving you an opportunity to trap them and handle them on your own. You might log them to a file, display a friendlier error message, or do something else to that effect.
Show Me Your Scripts
As I said, some of these have been a bit nitpicky—but they all offer distinct advantages that are worth consideration. I invite you to share your own Windows PowerShell scripts. I review a couple each month and post my suggestions on ConcentratedTech.com. Feel free to look up some of my past suggestions and maybe even submit your own scripts for review. I look forward to hearing from you!
Don Jones is a co-founder of Concentrated Technology. You can read tips on Windows PowerShell and other IT topics, as well as contact Don, on his blog at ConcentratedTech.com.