coolOrangeLabs / powerGateTemplate

powerGate full functional sample ERP integration
MIT License
3 stars 3 forks source link

Add null-coalescing operator #155

Closed lustricker closed 3 years ago

lustricker commented 4 years ago

Description

I used this code in many projects of mine where I often had to check wether a property is null or nullOrWhitespace or other things. The below code accepts a scriptblock which contains the logic for the computations/comparison and returns either the value of the property or an alternative value. Similar to the null-coalescing operator in .Net.

Code

function ?? {
    if ($args.Count -eq 3) {
        $Operation = $args[0]
        $ParameterToVerify = $args[1]
        $OptionalReturnValue = $args[2]
    }
    elseif ($args.Count -eq 2) {
        $ParameterToVerify = $args[0]
        $OptionalReturnValue = $args[1]
    }
    else {
        throw "Function ?? (Test if null), wrong Argument count!"
    }

    if (-not $Operation) { $Operation = { param($p) ; $null -eq $p } }
    $badResult = Invoke-Command -ScriptBlock $Operation -ArgumentList $ParameterToVerify  

    if ($badResult) {
        return $OptionalReturnValue
    }
    else {
        return $ParameterToVerify
    }
}

Usage

$VaultItem.MandatoryProperty = ""

$NoW = { param($p) [string]::IsNullOrWhiteSpace($p) }
?? $NoW $VaultItem.MandatoryProperty "Take this value if null or whitespace"

If no scriptblock is provided the default check is if the argument is null. So it can be really short and readable like:

?? $VaultItem.MandatoryProperty "Take this value if null"

Discussion

What do you think about this? To complicated? I find this usefull to not have everytime:

if(-not $VaultItem.MandatoryProperty) {
  $VaultItem.MandatoryProperty = ""Take this value if null or whitespace"
}

To make better: Replace the $args parameter with some kind of Parameter[ParameterSetName = ""] to increase readability, but was not able to do this :D

lustricker commented 4 years ago

@christiangessner @PatrickGrub what do you think?

christiangessner commented 4 years ago

As a developer, I really like this approach. However, as an advocate for our partners (which are not developers but scripting guys) I think this is too complicated. I'd rather have a couple more lines of code that are simple and readable than having proprietary functions to deal with. Please, let's keep the powerGate template as simple as possible

lustricker commented 4 years ago

PowerShell 7 support the null-coalescing operator like we know it from .NET.

Since most of our projects are written in powershell 5, maybe we can introduce something similar like in the question above, but more userfrienldy. E.g. sending a value through a pipeline:


$isNull = $VaultItem.MandatoryProperty | IfNull-Then "Take this value"
$isNullorWhiteSpace = $VaultItem.MandatoryProperty | IfNullOrWhitespace-Then "Take this value"

I would find this usefull, since for instance in the PrepareErpMaterial function, there might be a lot of if(-not $null) statements, and this makes the code "complicated" aswell....

PatrickGrub commented 3 years ago

I really like the approach from https://github.com/coolOrangeLabs/powerGateTemplate/issues/155#issuecomment-691935998 How about you @christiangessner ?

christiangessner commented 3 years ago

I still believe that our customers and resellers understand the "if(-not $null)" syntax and don't see a good reason to change this.

PatrickGrub commented 3 years ago

I had experience in a mentoring with c# developers where it was not intuitive if(-not $null) for them. Also @kieninj stated multiple times that the automatic conversion of PowerShell is not intuitive for type-oriented programmer like C#

I think the approach from Lukas https://github.com/coolOrangeLabs/powerGateTemplate/issues/155#issuecomment-691935998 would shrink the code and complexity.

Code like this:

$userInput = $dsWIndow.FIndName("TxtBx").Text
if(-not $userInput) {
  $userInput = "My default value when empty"
}

Would be come like:

$userInput = $dsWIndow.FIndName("TxtBx").Text
$userInput = $userInput  | IfNullOrWhitespace -Then "My default value when empty"

The only contra that I see is PowerShell version 7. Since that's not the default with windows 10, so users would make separately the installation and this could cause more complications for instance in restricted customer projects. Therefore I would wait for PS 7 to be standard in Windows and change our codings.

Would you agree @christiangessner ?

christiangessner commented 3 years ago

I would not change anything in our code