Intro
A few weeks ago, someone in the slack channel asked what some of the Code Smells are in PowerShell. If you are not familiar with the term Code Smell, a simple definition is code that when you see it you know there will be issues just like when you smell something rotten know you will find something unpleasant. After some joking around the term PowerSMell was coined to describe a Code Smell particular to PowerShell.
This is the first in an ongoing series of Code Smells in PowerShell. Rather than make a massive single entry, I will try to cover only a few at a time and spread them out. Unlike my other series, this one wont be dumped all at once and I will work on this series between other blog posts. I may never truly be done with this series and I have no clue how many parts it will be. There are many Code Smells in PowerShell and it seems like I'm finding more each week.
One possible plan for the future is to begin a community PowerSMell project with PSScriptAnalyzer rules for linting these PowerSMells. I know there are several projects out there that already have some of these, but it would be nice to get them all together. If you are interested in working together on this let me know and we can begin coordinating.
Each PowerSMell will include a "The Smell" and a "The Source" section. The "The Smell" section will have a code example of the PowerSMell. The "The Source" section will have information about the problems that the smell hints to. Some PowerSMells are not always PowerSMells. When a certain PowerSMell has a non-smelly usage, it will include a "The Pleasant Aroma" section providing an example of the non-smelly usage.
Table of Contents for this series
What are Code Smells?
Have you ever walked into a room or building and smelt something absolutely disgusting or very weird? If so, what was your first thought? I venture to suppose that you have and that your first thoughts were either "what is that smell", "what is making that stink?", or something similar.
A smell isn't a problem in an of itself, but rather a symptom of a problem. Something is rotten, dead, dying, putrid, or dangerous. That is the real problem. The smell is just alerting you to it.
Code Smell is similar. Code Smell isn't necessarily the problem, but is often (though not always) a good indicator that there will be problems. Code Smells hint at bad programming habits or a lack of understanding by the coder. Code Smell exists in every language in one form or another. Code Smells are not necessarily bad code. In fact, Code Smells could be completely legitimate code, just in the wrong place at the wrong time (just as some smells are great in the kitchen, but probably not so great in the bedroom).
For the sake of this series, assume I am only talking about actual scripts and not console activities. Some PowerSMells are perfectly legitimate when hacking away at a console but are Code Smell in scripts. I always preach that all bets are off when it comes to form, syntax, and best practices in the console. Scripting is a different story. Scripts are long lived and often need to be maintained by more than just the person who wrote them. Best practices matter in scripts.
PowerSMell001: Semicolons ";"
Semicolons are probably the most common PowerSMell and easy to spot.
The Smell
Here are some semicolons PowerSMells:
The Source
Semicolons are perfectly legitimate for terminating statements and expressions in PowerShell. They are, however, completely optional unless you are trying to do multiple commands on one line. In a script, you should not be trying to do more than on command per line. This PowerSMell hits at a couple of problems.
First, semicolons are mandatory expression terminator in several languages and that means the coder may not be acclimated to PowerShell. Those of us who come to PowerShell from other languages tend to bring our knowledge and habits with us. In some cases this results in code that is anti-pattern or, at least, riddled with false assumptions about PowerShell. This results in sophisticated land mines buried in the code that appear as normal PowerShell but end up causing problems.
Second, since semicolons are used heavily in console use to make one-liners, semicolons hint at this code being refactored from a console session. The problem here is that console cleverness is often scripting terrorism. The presence of semicolons means that there may be some poorly refactored code about that still holds to its console roots.
Third, semicolons hint at a lack of best practice adherence. Semicolons shouldn't be used outside of string literals in a script. They may not be ultra-critical to RBP adherence, but their presence hints at either a lack of knowledge or complete disregard for PowerShell best practices. This means the code will likely be painful to modify or maintain.
For loops may be an exception to this code smell, but, often for loops are a PowerSMell.
PowerSMell002: Plus-Equals +=
The plus-equals incrementing operator is another common PowerSMell.
The Pleasant Aroma
Plus-equals is not always a PowerSMell. There is one particular area where this is perfectly acceptable and that is with regard to numbers:
The Smell
Here are some plus-equals PowerSMells:
The Source
The problem with plus-equals is one of portability. Not all collection types implement it. Also there is the issue of arrays being a fixed size and plus-equals is effectively building a new array with one more element than the previous one and then populating it. It will also coerce other collection types, such as ArrayList into object arrays. Seeing plus-equal in use with collections indicates a lack of understanding of .NET data structures. That's not bad, necessarily, but it could result a serious performance impact with larger collections. Plus-equal hints at code not being built to scale.
When plus-equal is used with strings, it hints at a novice coder. Plus-equal has a funny way of not working in expected ways with objects you think are strings but actually are not. You can get around this by strongly typing your string variable, but strongly typing variables is not necessarily a good PowerShell practice. There are better ways to concatenate strings such as the -f format operator or [System.Text.StringBuilder] and a dozen others. Plus-equal with strings means that there may be hidden land mines in the code when an object is not a string. These can be hard to catch until it's too late.
PowerSMell003: Function Verb-Noun ($Parameter) {}
There are quite a few ways that can be used to define functions in PowerShell. One form in particular stinks.
The Smell
Here is the function definition code that smells (pay particular attention to the placement of the parameter):
The Source
This particular definition method creates "simple" or "basic" functions. It's not the only way to create simple functions, but this form smells of someone coming from another language. For clarity, I'm speaking specifically about the exact form used above and not about simple functions in general. This definition method is very similar to how functions are defined in other languages. People coming from other languages often bring their assumptions with them. The result is that those assumptions can create hidden land mines that go off unexpectedly due to the coder's lack of understanding of PowerShell.
This smell is often coupled with another smell: using return in a function to return values. I will cover that smell in a later post.
"Simple" or "basic" functions in general can also be a PowerSMell too. They have their uses. In my experience, more often than not they are not used correctly. I will also cover this PowerSMell in a future post.
Conclusion
That's it for this edition of PowerSMells. There are definitely more to come, so check back in a few weeks.
Do you have a PowerSMell you want to see covered here? let me know and I will add it to the list. The best ways to contact me are either through Twitter (@markekraus), The Poshcode Slack (@markekraus there too), or on Reddit (/u/markekraus). Like wise, if you want to collaborate on creating a PSScriptAnalyzer module for linting these PowerSMells, reach out!
