Add Style Guideline Example for How Braces in Switch Block should be Used

[PlagueHO]

There is currently no example of how braces in a switch block should look.

Options:

1. switch ($condition)
   {
   'a' { Statement }
   'b' { Statement }
   }

2. switch ($condition)
   {
   'a'
   {
     Statement
   }
   'b'
   {
       Statement
   }
   }

3. switch( $condition )
   {
   { $_ -eq 'a' -or $_ -eq 'b' }
       {
           Statement1
       }
   { $_ -gt 0 -and $_ -lt 10 }
       {
           Statement2
       }
   { $_ -match "regex" }
       {
           Statement3
       }
   }

@mhendric, @johlju - your thoughts? Should we add this and what is the preferred method. I've used 2, but only because I assumed that was covered - which it isn't. I'm good with either way.

I think 2 is easier to enforce because 1 will result in two styles potentially:

1. switch ($condition)
   {
   'a' { Statement }
   'b' { Statement }
   }

   and

2. switch ($condition)
   {
   'a' {
     Statement 1
     Statement 2
   }
   'b' {
     Statement 1
     Statement 2
    }
   }

What is everyone's thoughts?

[essentialexch]

You left the style necessary for complex cases. For example

switch( $condition )
{
    { $_ -eq 'a' -or $_ -eq 'b' }
        {
            Statement1
        }
    { $_ -gt 0 -and $_ -lt 10 }
        {
            Statement2
        }
    { $_ -match "regex" }
        {
            Statement3
        }
}

And here you see my preferred style too. :-)

[PlagueHO]

Thanks @swngdnz - added to the list :grin:

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity from the community in the last 30 days. It will be closed if no further activity occurs within 10 days. If the issue is labelled with any of the work labels (e.g bug, enhancement, documentation, or tests) then the issue will not auto-close.

[PlagueHO]

Bump

[gaelcolas]

I'm not a fan with enforcing this, as different usage of the switch statement might call for different layout. In some (rare) cases it's much more concise and readable to to have a single line for the case statement:

$value = switch($blah) {
    'a'        { 'A' }
    1          { 'A' }
    default { 'B' }
}

My point is that we should recommend some format, but probably let the maintainers decide if they want to enforce a way or another in their repository.

[gaelcolas]

Just came across an Illustration of the above in a PR:

I personally prefer the former layout... It does against vscode auto-formatting feature though, so I'm not that bothered, I can adapt.

[johlju]

We have the style guideline that says open brace should be in a separate row, so then the code gets stretched out in scenarios like these. I think having the style guideline as-is is good because we have one way of writing the code. Of course there are a lot of different ways to write code, but the if we should improve review time by adding PSSA rules for style, then I don’t think we can relax the rules in one area but not in another. Well we can if we the community choose of course. And using VS Code auto-formatting is a time saver too, and then if a contributer was write the open and closing braces differently the auto-formatting would add them back and making more changes than the PR intended.

I’m always open to style changes if it improves that everyone writes the code the same and supports “easily written” future PSSA rules and auto-formatting.

[PlagueHO]

I do prefer the more concise layout at @gaelcolas mentioned. But only if there is a single line of code in each block. So perhaps the rule could be "Compact" when only a single line in each block is used. E.g.

$value = switch($blah)
{
    'a'        { 'A' }
    1          { 'A' }
    default { 'B' }
}

And expanded otherwise:

$value = switch($blah) {
    'a'       
    {
                Write-Verbose -Message 'Line 1'
                Write-Verbose -Message 'Line 2'
    }
    1
    {
                Write-Verbose -Message 'Line 1'
                Write-Verbose -Message 'Line 2'
    }
    default
    {
                Write-Verbose -Message 'Line 1'
                Write-Verbose -Message 'Line 2'
    }
}

That said, sometimes we use switch blocks when we should possibly have defined an enumeration instead. So that kind of block may be an indicator that some refactoring could be made.

I'd suggest thought that this isn't allowed:

$value = switch($blah) {
    'a'        { 'A' }
    1          { 'A' }
    default { 'B' }
}

[gaelcolas]

My example was only to illustrate that enforcing is not necessarily better than recommending (though, agreed, it's a 1-line content edge case). But I agree with what @johlju pointed out, it might be easier to just enforce what's configurable in vscode, and preferably the defaults to avoid formatting hell in PR reviews.

As for having one and unique standard across all repositories (for cosmetic changes that don't affect quality), I'd argue that it mainly serves you both (not many people are doing as much work across so many repositories). I'm not saying it shouldn't be the case and I do want to make it easier for your eyes!

I also agree that it helps people contributing to multiple repositories, and it should be enforced by the maintainers in any given repository.

But it repulses some would-be contributors/maintainers (I had this direct feedback), not in existing repository because people happily comply with established standards, but for new resources they want to share and include in the Resource Kit.

In conclusion, I'd say that unless we can configure specific auto-formatting in vscode (aligned with options 1,2,3) and someone to ask for it specifically, I'm happy to have the guidelines updated and enforced with option 2 and 3 only as suggested by @PlagueHO.

Should we bring this update to next Community call?

[johlju]

I’m not sure we can ask the repositories outside of DSC Resource Kit that they must enforce the formatting guidelines. I know we say they should follow the guidelines, but that should be just that, a guideline. Maybe they can just run there own guideline which overrides the “central” one.

But repos owned by the PowerShell GitHub account I think should enforce the guideline for all PRs.