With Event 1 in the books for the 2013 Scripting Games, we are now in the voting period where the community gets the chance to play judge on all of the scripts submitted by voting and commenting on the submissions. My next articles will cover my “Favorite” and “Least Favorite” scripts submitted along with my reasons why. For this article, I want to cover some of the things that I have seen which are not the best approach when writing your script/function.

Where is the help!?!

Yes, where indeed is the help at? I was amazed by just how many submissions I have reviewed that didn’t have inline help (comment based help).  For the advanced entries, there really is no excuse to why no help has been added. By the way, this is not acceptable help:

Function Test-Something {
    <#
        Script does something

        Usage: Test-Something -Computername H1 -Source C:\
        
        More stuff here    
    #>

}

There is a specific format to the help that you need to use to make it work properly.

Same goes for when help is included, but missing key things such as parameters. If you have a parameter listed in the function, it had better be listed in the help. I saw one example that looked something like this:

Function Test-Something {
    <#
        .SYNOPSIS
            This does something

        .DESCRIPTION
            This does something

        .PARAMETER Computername
            Name of computer

        .PARAMETER Source
            Source of something

        .EXAMPLE
            Test-Something -Computername H1 -Source "C:\" -Destination "D:\"
            
    #>
    [cmdletbinding()]
    Param (
        $Computername,
        $Source,
        $Destination
    )
    ...
}

Where is the Destination parameter in the help? This really is the easiest part of the script. Just add all of your parameters, give a decent description and some good examples (in fact one of your examples could be based on the requirement of the event!).

Naming conventions for your functions and scripts

I actually have an article out for this here. Short story: Use Verb-SingularNoun for your names. That is all.

Aliases…Don’t use them

While it works great for one-liners and when you might be working interactively, using aliases is a great way to work with PowerShell. What it isn’t so great for is when you are writing scripts (and especially when you are submitting scripts for the Scripting Games!). If your code looks like the below example, then you need to re-think your strategy.

GCI C:\Application\Log -Recurse -Filter "*.log" | `
?{$_.LastWriteTime -lt (get-date).AddDays($TimePeriod)} | `
%{$Dest=$_.Directory.Name

This is difficult to read unless you know what all of the aliases mean. In this case, it might not be that bad, but imagine hundreds of lines of code that you are working through to troubleshoot. Are you really going to want to do this? I wouldn’t want to for this and another reason that segues into this nicely…

Formatting your code

What else is wrong with the above code? If you said the backtick “`”, then you are correct! This is not a good way to break a line in the code. Use what PowerShell has provided for you in the following: pipe “|”, opening curly bracket “{“ and comma “,”. Each of these servers as a natural line break that allows you to continue onto the next line without worrying about errors occurring. The same goes if you an opportunity to break a line before it gets too long.

Get-ChildItem C:\Application\Log -Recurse -Filter "*.log" | Where {
        $_.LastWriteTime -lt (get-date).AddDays($TimePeriod)
} | ForEach {
    $Dest=$_.Directory.Name
}

Easier to read and troubleshoot. Remember, it might look good now, but think about who else could be looking at your code and the possibility that you will be back to look at it in 6 months when you need to troubleshoot or add something to the existing code. Might not look so pretty then.

Use parameters instead of Where-Object for filtering

The parameters are there for a reason. They can more efficiently filter out the good data that you are looking for from the bad data. This especially goes if you are writing your scripts for PowerShell V3. Take this example:

Get-ChildItem -Path $Source -Filter *.log | Where {$_.PSIsContainer}

While I’m happy that the Where-Object is being used correctly, being that this is V3, it is completely unneeded. Get-ChildItem has a –Directory parameter that filters out for only Directories. Same goes for files with the –File parameter. This practice should be used on any event or “real world” script you write.

Useful Parameters

While we are on the subject of parameters, make your parameters useful. Look at what exist already in PowerShell for parameters and make those your baseline. Computername, Source, Destination, etc… should be what you aim for. Not Computer, Server, DestinationFolder,A,B,C etc… If something similar doesn’t exist, then keep it singular when you create it, not plural. Strive to keep consistency and it will pay off for you!

This is PowerShell, not Vbscript

Please do not follow a vbscript mentality. When you create a variable, make it meaningful to what you are saving, not the type of object you are saving. Any of these are bad:

• strcomputer
• arrnumbers
• objwmi

Just keep it to what the variable represents instead of the type of data and you will be following a better practice of writing scripts in PowerShell.

The same goes for the “catch all” error handling that would occur in vbscript with On Error Resume Next. Do not use $ErrorActionPreference=’SilentlyContinue’ at the beginning of your scripts. This will mask every error that occurs and will usually work against you, especially when troubleshooting why the script isn’t doing what you think it should.

Accepts pipeline input, but where is the Process block?

If you have a script with a parameter that you have decided to accept pipeline input, then you had better have your Begin,Process and End block (or at the very least a Process block; but you had better have EVERYTHING in that Process block or it cause you issues!). If you do not have a Process block, then you are really missing out on the purpose of doing allowing pipeline input. Check the following out:

Function Test-Something {
    [cmdletbinding()]
    Param (
        [parameter(ValueFromPipeLine=$True)]
        [string[]]$Computername
    )
    ForEach ($Computer in $Computername) {
        $Computer
    }
}

Let’s run this and see what happens with pipeline input.

1,2,3,4,5 | Test-Something

What do you think will happen? Will it show everything or will it just show the last item? Lets find out!

Well, how about that! It only display the last item in the collection that was passed to the pipeline. Why did this happen? In short, it was because we didn’t use a Process block to handle the pipeline input correctly. Lets fix that!

Function Test-Something {
    [cmdletbinding()]
    Param (
        [parameter(ValueFromPipeLine=$True)]
        [string[]]$Computername
    )
    Process {
        ForEach ($Computer in $Computername) {
            $Computer
        }
    }
}

Now I will run the same command and lets see what happens this time!

Now that is much better! It is now behaving exactly like it should with the pipeline input. Remember, the process block will run for each item being passed through the pipeline, so do not keep anything in that block that you plan on using at the end, such as creating an empty array for a report to store each item being processed (yes, this did happen in a submission).

Function Test-Something {
    [cmdletbinding()]
    Param (
        [parameter(ValueFromPipeLine=$True)]
        [string[]]$Computername
    )
    Process {
        Write-Verbose "Processing pipeline"
        ForEach ($Computer in $Computername) {
            $Computer
        }
    }
}

Use the Begin{} to initialize anything that you need prior to the Process{} block and then use End{} to wrap anything up.

I think I have covered enough things for one night. Keep these tips in mind as your not only progress through the rest of the events, but work on future scripts out in the ‘real world’.