Skip to content

Conversation

@sethvs
Copy link
Contributor

@sethvs sethvs commented May 2, 2018

PR Summary

-Not parameter is very ambiguous.
Is it better to rename it to -IsNullOrEmpty?

Also, ParameterSetName is changed to 'IsNullOrEmpty'.

PR Checklist

@sethvs sethvs requested review from BrucePay and daxian-dbw as code owners May 2, 2018 09:37
@sethvs
Copy link
Contributor Author

sethvs commented May 2, 2018

Test-Connection.TraceRoute.TraceRoute works AppVeyor test failed.
Should we restart tests?

@iSazonov
Copy link
Collaborator

iSazonov commented May 2, 2018

-Not parameter was approved in #6464 - should we rename it?

/cc @SimonWahlin @SteveL-MSFT @mklement0

@mklement0
Copy link
Contributor

@iSazonov:

No, -IsNullOrEmpty is not appropriate, because -Not has a wider scope, applying to [bool] values too:

[pscustomobject] @{ Foo = $False } | Where-Object -Not Foo   # OK, passes the input object through

@sethvs
Copy link
Contributor Author

sethvs commented May 2, 2018

@mklement0
Concerning creating an alias -IsNullOrEmpty for -Not parameters #6796 (comment) - this is IMO a totally different meaning and it will be even more confusing than now.

Agree, -IsNullOrEmpty is not ideal for [bool], but -Not has far wider meaning.
For example, it seems totally natural to use it like this:

ps | where name -Not -Eq 'pwsh'

So I think there should be better parameter name.

@sethvs
Copy link
Contributor Author

sethvs commented May 2, 2018

I believe -IsFalseNullOrEmpty sounds horribly :)

@iSazonov
Copy link
Collaborator

iSazonov commented May 2, 2018

Where-Object is a bool filter - I don't see any problem with -Not.

@daxian-dbw
Copy link
Member

I agree with @mklement0. -Not also applies to the numeric value 0:

[pscustomobject] @{ Foo = 0 } | Where-Object -Not Foo

I don't see a problem with -Not. It aligns with the -Not operator, which is consistent.

@mklement0
Copy link
Contributor

mklement0 commented May 2, 2018

@sethvs:

Sorry for creating confusion: in #6796 (comment) I was speaking in the abstract, giving general advice: you shouldn't just rename existing parameters, because that is a breaking change; if appropriate, a parameter alias can be implemented - but I do not think that that is appropriate here, for the reasons stated by me earlier and also by @iSazonov and @daxian-dbw.

Note that -not is a unary operator in PowerShell, so even in the context of an expression something like $name -Not -Eq 'pwsh' is a syntax error.

The -Not parameter, as the -not operator analog, works as advertised - I don't think it needs another name.

With strings, -Not works like [string]::IsNullOrEmpty(), but, as stated, -Not also works with Booleans and numbers and generally coerces anything to a Boolean based on the concept of "emptiness".

You can think of -Not ... as [bool] (...) -eq $False.

@sethvs
Copy link
Contributor Author

sethvs commented May 4, 2018

OK.
Not totally convinced but closing.

@sethvs sethvs closed this May 4, 2018
@sethvs sethvs deleted the where branch May 21, 2018 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants