PowerShellPracticeAndStyle
PowerShellPracticeAndStyle copied to clipboard
PoshCode
Should Parameters Always Have Default Values?
I've already written about this, but it's probably worth having this conversation here.
Function parameters are always automatically defined in the local scope and initialized by PowerShell to their default value (the equivalent of null -- which comes out as an empty string or a zero for numerical values, or a default struct, etc).
This happens even when they are part of an unused parameter set.
There is, in my opinion, nothing to be gained in setting them yourself -- unless you actually need them to default to something other than $null.
If anything, this should be warned against for performance reasons, and because people frequently provide default values on mandatory parameters (which can be misleading and even invalid defaults, since they're not used).
As with C# and most other programming languages, initializing a variable to it's default value is just extra work that accomplishes nothing. It might even make your script (infinitesimally) slower, whilst changing nothing.
Agreed. Although Tobias brings up a fair point about folks using non-mandatory without default values later in their script without ensuring definite assignment first - resulting in potential $null reference errors.
If there isn't already a PSSA rule for this, I'd love to see one added. But even without the rule, I would encourage null-checking on non-mandatory parameters over assigning $null as def value - which actually doesn't help with the $null reference problem at all except that perhaps the scripter will see that the default value is $null. But that doesn't change the need for null checking later as the scripter has no idea whether the function caller provided a value for that parameter or not.
There are two issues to this IMHO. Technically, it is not necessary to assign a default value to an optional parameter if you are fine with the default $null value, nor is it always possible (as with DateTime type as was pointed out). Assigning $null to an optional parameter as a default value does produce better manageable code, though. While truly experienced PowerShell writers know about the default assignments, mere mortals in the field often do not. When assessing production code, very often there are functions that declare optional parameters that need values other than null. They cannot use $null. If the author was asked to either assign $null as default, or mark the parameter as mandatory, the author would be forced to think about it and quickly identifies parameters that cannot use $null and should be mandatory. So the rule "optional parameters should have a default value" is not a technical requirement. It is a rule designed to pinpoint a potential issue.
Commenting on c#: When you write a method in c#, you need to define parameters. You can either provide an explict default value (in which case the parameters are optional), or provide no default value (in which case they are mandatory). What you cannot do is define a parameter without default value and then omit it. It is different with fields. They are initialized to null/0/false. However, "publicly" accessible structures such as methods (in c#) and functions (in PowerShell) should leave no doubt what their respective value should be. The fact that PS uses field initializer for optional parameters with no default value IMHO is just something to make PowerShell more robust, but not a best practice that should be recommended.
When assessing production code, very often there are functions that declare optional parameters that need values other than null.
Absolutely and that is the case where you do want to use a default value for a parameter.
Assigning $null to an optional parameter as a default value does produce better manageable code
I don't agree on this because you still need to null check the parameter before using it. Assigning $null as a default value does not change this.
But @rkeithhill, that's not a valid point. If you need to set the default to something other than $null, then of course you should do that. But setting it to null doesn't accomplish anything (it already was null, that's why you hypothetically had that problem). You're merely invoking an assignment to no purpose.
@rkeithhill as I linked in the OP -- there used to be a rule. It was wrong. I convinced them to ditch it.
@TobiasPSP always assigning default values (and especially adding rules about it) is misleading your "mere mortals" -- you're making them think that they need to do things they do not. They need to learn that PowerShell parameters already have default values -- not learn that they have to be assigned (when they do not!).
In PowerShell, ALL PARAMETERS are initialized. EXACTLY THE SAME WAY that in C#, all variables are initialized.
Assigning $null to a default value is an active measure you took, so chances are you thought about it. Not assigning anything is often something that just slipped through.
@Jaykul that is not true. C# does not work the way you suggest. Compare method parameters, not field initializers.
It is absolutely incorrect that "mere mortals" think about what PSScriptAnalyzer tells them to do. It says assign a default value, they just do that.
@Jaykul If you need to set the default to something other than $null, then of course you should do that.
That's what I thought I said. Sorry if it wasn't clear.
@Jaykul in the real world, we have uncovered tons of issues this way. People write functions and define parameters, then they call them always with their sets of arguments. Once they export this to module and make it public, people start to submit different arguments (or none), and all breaks. The rule helped people immediately find their issues.
@TobiasPSP: you can't compare method parameters, because in C# method parameters are mandatory. Assigning them a default value is the syntax for making them optional, it is not to make them have a value.
Compare method parameters, not field initializers.
It's not about the specific ways each language handles parameters. It is a more general concept of - don't add extra script (more script, more bugs) that is unnecessary.
So why strongly type a variable? If you always write perfect code, you can get around a lot of things that otherwise can protect you.
I guess what I want to say is: there is a natural clash when it comes to best practice discussions. Everyone has his or her special background, and of course we all think we know what is best. That's why rules can be disabled. No hard feelings. My focus is on the field. I am at companies every single week, working with thousands of ITPro during a year, reviewing tons of code, most of it pretty ugly. I want rules to be practical, and to produce better and safer code. When they do that, I am happy.
So why strongly type a variable
Because I'm giving PowerShell information it didn't have i.e. that I want this variable to not be type promiscuous as Bruce would say. :-)
The point is that you're suggesting people write a variable assignment as "documentation" But what you're documenting is "how PowerShell works" ... not "how my function works"
The only documentation you need of default values is [Parameter(Mandatory)] to indicate when they are not.
So when a rule proves to identify significant numbers of real-world issues, then you are still opposed to it because people should have just written better code in the first place? Hm, maybe I did not get the idea of these rules then :-)
I want rules to be practical, and to produce better and safer code. When they do that, I am happy.
I still think this could be accomplished with a PSSA rule that verifies the scripter has null checked a non-mandatory parameter in script before reference it for any purpose (other than assigning to it -which would be weird and probably another rule violation). @Jaykul are you sure about the rule you got them to remove? I thought that was just to ensure parameters have default values assigned which is different than what I'm proposing.
It took you only seven posts to switch from trying to convince me there's a good reason for this to claiming you know best based on your particular background. Not helpful. That's what a Thumbs Down is.
what kind of comment is a "thumbs down"?
On GitHub you can "react" to someone's comment with a thumbs up/down, yay, etc. Check out the "+smiley" in the upper right of a comment's toolbar .
@rkeithhill null-checking would be equally valuable in uncovering the issues I talked about. Maybe it would also be a better approach. @Jaykul the purpose was a discussion I assumed. Now it is ideological and personal. I apologize if I did something wrong, was not my intention.
I especially did not want to appear as "I knew best", I just wanted to illustrate that there is more to this than just a highly technical approach.
@rkeithhill null checking is complicated in PowerShell because you can do it in several ways, however, you are right that a rule which actually checks the code would be very useful, and is not what they had before.
true, the challenge probably would be how you identify whether the PS code in fact did a null checking.
null checking is complicated in PowerShell
I've been too spoiled by C# definite assignment checks. :-)
Can I ask a final technical question? What (other than more code than needed) are the downsides of assigning an explicit default value to optional parameters? After all, most of the time this is what is done anyway (most of the time, the default value is non-null). Trying to understand the pros and cons.
@TobiasPSP: I spend most of my day on Slack and IRC in the PowerShell user group channel. I spend hours each day teaching people PowerShell there. I know that there are many different levels of skill, and many different types of people with different backgrounds learning PowerShell. However, my exposure to how people think and code isn't the point. The point is whether what we should be teaching them.
I am suggesting that what we should be teaching them is that:
- Variables (and especially parameters) have default values.
- What those values are depends on the variable type (of course you can assign a default to a DateTime, just not
$null) - If you want the user to choose the value, you need to use
[Parameter(Mandatory)] - If you know a better and acceptable reasonable default value, you may set that
- Validation attributes don't run on parameter default values ❗❗
- But type coercion and ArgumentTransformation attributes do
-
$PSBoundParameterValuesis your friend, it's the only way you can tell if a user passed the same value as the default value.
@Jaykul completely agree. Here is what I am struggling with: We don't have the chance to always teach people these things. How can we support them through pure code analysis?
As you and @rkeithhill suggested, analyzing PS code to find out whether there is appropriate null checking would be the technically best way but is probably prohibitively complex. In addition, when you think about it, we don't just need $null checking, we then need "validation" validation, thus need to check whether any default value is suitable, not just $null. That's out of scope I suppose.
When you look at the source of most problems, it starts IMHO here: many novices just define the parameters for the arguments their code needs. They don't consider the case where arguments are not submitted, nor do they see that this can create unexpected $null values.
I hope you see that I did not want to brute force my view. I am struggling to find a manageable way to solve this, just like you. The perfect rule might be: "always assign an appropriate default value to optional parameters when the default value should not be $null".
But since you cannot detect whether $null is or is not appropriate, the second best rule would be "always assign a default value to optional parameters", considering that $null seldom is the default value that people come up with in the end, and paying the price that there might be some $null assignments that are not really needed.
So what is better?
- having some rare cases of $null default assignments where they are not really needed
- having no option to point novices to potentially undefined optional parameters through automated rules at all
I agree that technically, assigning a $null value is not a good recommendation. When you do, though, you explicitly "declare" that you are ok with it, and enable rules to find all the other cases. That's why I thought it might still be a good idea.
Let me preface this by saying that I heartily endorse the use of reasonable default values for non-mandatory parameters -- the more you can do by default, the better. In particular, I love the fact that I can have a default value for a parameter that uses values from other (mandatory?) parameters. e.g. this is wild
@TobiasPSP the main downsides, as I see them:
First, it's unnecessary code - you're (re)doing something that's already done. The less you write, the less you have to maintain.
This does not just mean extra work on your part (to type it). It also means extra work on PowerShell's part every time it's run, including type coercion (e.g. $null to int = 0).
It also means people who come after you have to read what you wrote, and figure out: Why did he assign it the value it already had? They have to worry about whether they're misunderstanding something, did they misread the type? Is there an argument transformation attribute that this is triggering?
Second, it's possible to initialize parameters to values that are not allowed by the Validation attributes. This can cause all sorts of trouble, and is more likely when people are just a checklist that says they must to assign some default value.
Third, it's skipping over teaching people how PowerShell *actually works. It misleads people into thinking they really do have to assign a default value. It makes them believe that this is like Visual Basic (or whatever other language they came from) where they have to initialize everything ...
Additionally, there's the case that struct parameters are always null by default, but cannot be assigned null.
As soon as you get a DateTime or a Rectangle or any other struct (in general, non-numeric ValueType descendants), you cannot assign it null, but if you just leave it alone it will be null. What would your "always assign a default value to optional parameters" rule do here? It will force people to assign something silly and then write a silly test for it. (i.e. testing DateTime values are greater than January 1, 1970, instead of just checking that they're not $null).