Quit copying and pasting your scripts!

June 13, 2015 by Christopher Walker

And other PowerShell no-nos in my book

Though the majority of my time is spent writing scripts for various automation purposes, I am also frequently asked to review scripts. I harp on many things when returning my assessments but one of the biggest factors I tend to dwell on is reuse. As I review, one thing I notice is the method people choose to write their code. I begin to notice patterns of the people I have reviewed previously. Several times I have found large snippets of script simply picked up from a previous script and dropped into a new one. Sometimes it’s not blatantly obvious that it was moved over, and most of the time, those tiny one or two lines are harmless. When it becomes entire portions of logic that do the exact same thing I have seen in a previous script, that’s when I start to twitch a little bit.

Just as waste products have the three Rs (reduce, reuse, recycle), coding has its own three Rs; reduce, reuse, refactor.

Reduce: Reduce the amount of code or complexity it takes to solve the issue while still being readable, maintainable, and without breaking the functionality. This is an ability developed over time. It is also developed as you learn the language of your choice. While I am fairly proficient at reducing code in PowerShell, I am not so proficient in Java. I haven’t spent the same amount of time on it as I have PowerShell. If you can’t yet understand the ways to reduce code in PowerShell, give it time and eventually you will learn more tools to help you do just that. Here are a few examples of code reduction in PowerShell that I have learned.

The first little snippet is something that took me an embarassingly long amount of time to figure out. When getting an object returned where you only need a few of the members, you typically use Select-Object. This returns just the member requested but it returns it in an Object[] (object array) of type PSCustomObject complete with the property name and the value.

    $ServiceNames = Get-Service | Select-Object Name
    $ServiceNames.GetType()

    IsPublic IsSerial Name                        BaseType
    -------- -------- ----                        --------
    True     True     Object[]                    System.Array
    
    $ServiceNames[0].GetType()

    IsPublic IsSerial Name                        BaseType
    -------- -------- ----                        --------
    True     False    PSCustomObject              System.Object

To get access to the value, I used to wrap the entire expression into parantheses and use the dot notation to retrieve the property:

    $ServiceNames = (Get-Service | Select-Object Name).Name

Some might say this was pretty concise but just imagine if that had a list of computers to pull from along with a Where-Object attached to it to pull only the running services. Wrapping all that in parentheses to get one property could be cumbersome for the reader and could be cumbersome for the writer in the future, especially if later down the road they decide they need more than one property. To resolve this issue, Select-Object has a parameter called -ExpandProperty and instead of returning an array of PSCustomObjects, it returns an array (or single value if only 1 object was returned) of the property type. In our Get-Service example, it would return an array of strings.

    # Quick tip: Use backticks (`), PS' escape char, to make piped lines 
    # more readable
    $ServiceNames = Get-Service | ` 
        Where-Object {$_.Status -eq "Running"} | `
        Select-Object -ExpandProperty Name

    $ServiceNames.GetType() 
                                                                               
    IsPublic IsSerial Name                        BaseType
    -------- -------- ----                        --------        
    True     True     Object[]                    System.Array

    $ServiceNames[0].GetType()

    IsPublic IsSerial Name                        BaseType
    -------- -------- ----                        --------
    True     True     String                      System.Object
    # notice that now it is string instead of PSCustomObject

Reduction of complexity is a necessity if any code is going to be maintained and used for long periods of time. The above is just one simple example of a problem I used to have. There are plenty of red flags for where reduction can occur.

  • Large if/elseif blocks: Usually, if I find myself more than two elseifs deep, I’ll revise all of those conditional selections to a switch. Switch statements in PowerShell are also very cool because when passing an array type, they automatically iterate through it for you. Just remember that unlike other languages, PowerShell’s switch does not have case fallthrough.
  • Accessing deep members of the same object: Consecutive lines where the code is accessing in-depth members of a larger object get a little tedious to maintain. For example:
    if ($some.deep.obj.member.hasValue -eq $true -or $some.deep.obj.member.isThere -eq $true) {
        Do-Something -Value $some.deep.obj.member.stuff
    }

    # A pattern can be seen here. Before you even go into the 
    # if block, assign a variable!
    $duh = $some.deep.obj.member
    if ($duh.hasValue -eq $true -or $duh.isThere -eq $true) {
        Do-Something -Value $duh.stuff
    }
    # Much more concise

I could harp on about reduction for days but it’s truly one of those skills that is picked up as you learn the language. So instead of beating a dead horse, let’s move on to …

Reuse:

This is the golden rule of programming. Reuse your code! And by reuse, I don’t mean:

  • copy this line and paste it three times, changing one thing for each line
  • copy this line and paste it two times, changing one thing for each line
  • copy this line and paste it one time, changing one thing for each line

See how ugly that is? Don’t do that. That’s not reuse or, at least, it’s not good reuse. Make a loop or better yet, a recursive function!

Code reuse is a simple concept to understand. Write modular code that does something very specific with a set of inputs and gives a consistent output. Write it so that it can be pulled into any future project and fit right in with existing resources.

Now, before you misunderstand me, understand this. You will eventually have to write code that directly applies logic to your desired end state. This is where common sense comes into play. Start code reuse practices when you notice yourself requiring a similar function from a script or program you have previously done. This is especially important in automation!

Prior to having access to the ActiveDirectory module and the Get-ADUser CmdLet, my buddies and I often found ourselves reusing (copy and paste reuse blah) a few lines of PowerShell code to reproduce the same functionality. On about the fourth or fifth script we were working on, I lost my mind and wrote a module that wrapped this functionality up along with some other useful, commonly used functions we had. After that, it was just a matter of importing the custom module and we had access to that function and several others. This allowed for easy access to the code without having to reach into another file, pull it out, and cram it into a new script. This also saved on maintenance time. Can you imagine if the module hadn’t been written but we discovered a flaw in our makeshift Get-ADUser? The maintenance nightmare that ensued would have defeated the entire purpose of automating in the first place.

There are many methods to go about reusing code:

  • If you find that code has been written previously to do the exact same thing you need, use it! Don’t reinvent the wheel! Places like CodePlex or GitHub already have vast amounts of code out there for the taking.
  • Specifically to PowerShell, write a module! If you don’t know how to do that, learn how to. Modules simplify your time in the console.
  • Write code that can be reused. Try to avoid writing code so specific that it can only have relevance to the script at hand. This known as hard coding. Avoid it like the plague.
  • Structure your code and if you are one member of a larger team, use source control and a common coding convention. A common code convention will allow for even simpler reuse since dragging in other member’s code will fit right in with yours.

Refactor: The majority of what I post about relates to scripting. When automating, the scripter often casts aside best practice to complete scripts in a timely manner. I can’t dispute this mentality because often there are tasks that need to be done quickly to ensure or restore uptime or contribute to service health. However, when the dust settles from whatever situation was causing you to hack away at the shell like you were playing WarGames, take a step back and review your scripting work. Review what you accomplished and determine if a similar automation process will be needed in the future. If you determine that code you wrote could be useful later on, save it and then refactor it.

Chances are, when you originally wrote a hasty automation script, you hard-coded values, relied on dangerous assumptions, and used quick one-liners to accomplish the task. Now that you plan on refactoring this code, yank those hard-coded values out, remove the assumptions, and expand the one-liners into actual maintainable expressions (if you know me, you know how I feel about one-liners).

While I can’t teach refactoring in a single post or even in multiple posts (there are entire books dedicated to the topic), I can give you an example I encountered where all three of these factors came in to play. I was asked to review a hastily written script to ensure that it would be useful on a mass scale. It involved mapping printers and specifically grabbing a network connected printer from a client, determining it’s current print server and remapping it to a new print server with a similar but slightly different name.

In this example, let’s say there were five old print servers, 01 through 05 and they were prefixed with OPS. There were also five new print servers, labelled the same way but with a prefix of NPS. Several safe assumptions could be made when I received the guidelines: - Each printer in the old print servers had a mapping to one of the new print servers - The printer name on the old server was the verbatim same on the new server - All the print servers were online and reachable via the network

There were several invalid (dangerous) assumptions that were also made and applied to the logic of the script: - If a printer was mapped to OPS01, it was also mapped to NPS01, the same for the other four servers. In reality some printers did not get mapped that way and were instead mapped to one of the other four - Printers were mapped to the client utilizing just the hostname. In reality some printers were mapped utilizing the FQDN of the print server

- The script also assumed that it always worked, so even on an error throw when the printer wasn’t truly remapped, the old reference would still get deleted, leaving no trace of the original printer.

The code I received contained a series of hard-coded if statements like below:

    #
    # code getting clients printer here 
    # and assignd to $RetrievedPrinterName
    #

    If ($RetrievedPrinterName.StartsWith("OPS01")) {
        #remap printer code
    } ElseIf ($RetrievedPrinterName.StartsWith("OPS02")) {
        #same exact copied and pasted remap code
    } ElseIf ($RetrievedPrinterName.StartsWith("OPS03")) {
        #same exact copied and pasted remap code
    } ElseIf ($RetrievedPrinterName.StartsWith("OPS04")) {
        #same exact copied and pasted remap code
    } ElseIf ($RetrievedPrinterName.StartsWith("OPS05")) {
        #same exact copied and pasted remap code
    }

Obviously this is not an ideal method but again, it was written hastily. The first order of business I took care of was making a function that performed the simple task of remapping one printer to the other. It took two strings as input, the name of the old printer and the name of the new printer. This allowed for a single point for the remapping. If the code to do the task needed to be changed, it was conducted in one spot. This also pulled out the hard-coded portion of the script’s logic. The function assumed nothing other than it was receiving two names. If the remap failed, it was left to throw an error to be handled later.

In the logic to acquire the printers, hard-coded values were eliminated and replaced by a regular expression which determined if an old printer was mapped via hostname or FQDN and also captured the server name’s number portion, which helped eliminate the need for the plethora of chained selection expressions. When the remap function was run based off a printer name, if an error was caught, the printer was not deleted thus leaving a trace for future troubleshooting. Although unimplemented, the solution for mitigating the first dangerous assumption was to catch the error and then loop through the other four servers till the remap was either successful or had run out of new print servers to remap to. In the end, the refactored code became more concise and was easier to maintain. It also resulted in the remap function which can now be used by all the technicians to perform the simple task of remapping a printer.

    Function Update-PrintServerMapping() {
        Param(
            [Parameter(Mandatory=$True, Position=0)][String]$CurrentPrinterName,
            [Parameter(Mandatory=$True, Position=1)][String]$NewPrinterName
        )
        #Remap logic
    }

    $RegEx = [Regex]"OPS(\d\d)(\.server\.domain\.com)?.*"
    #Logic to change name 
    Update-PrintServerMapping $OldName $NewName

Conclusion: This was a rather long article to follow along with but hopefully it will help with organizational automation. Reducing, reusing, and refactoring scripts allows technicians to focus on the important aspects of getting a task completely automated, cutting down the tedious, repetitive tasks that may plague their work schedule. Reduce code when you find that a script becomes cluttered. Brevity is key when writing automation scripts. Reuse code when it is possible. The entire purpose of information technology is so we don’t have to reinvent the wheel; allow your technicians to apply that same mentality to their automation. Refactor when necessary. Don’t trash entire scripts when you think there is something salvageable.

I have a few more items I’d like to write about for this series of posts. Be on the lookout for more coming soon.

Privacy Policy

© 2017 | Powered by Hugo and lots of motivation.