pokeheartgold icon indicating copy to clipboard operation
pokeheartgold copied to clipboard

Style: Indent level in switch blocks

Open PikalaxALT opened this issue 1 year ago • 10 comments

Variant A: Case labels flush with the switch statement. This is the standard in pret/pokeruby etc.

switch (state) {
case 0:
    // ...
case 1:
    // ...
}

Variant B: Case labels indented by 1.

switch (state) {
    case 0:
        // ...
    case 1:
        // ...
}

PikalaxALT avatar Aug 19 '23 23:08 PikalaxALT

Variant A is better due to how many nested switch statements are in the code. The alternative would be to have the deepest indent in large essential functions be 3-4 indents deeper than they would have been sticking with variant A, which makes the code less readable. Then for consistency we should keep the rest of the code on the same style.

adrienntindall avatar Aug 20 '23 04:08 adrienntindall

I have made several PRs converting style A into style B, which have been approved and merged, and during original style guide discussions, we did decided on style B, so imo style B is better, plus, I prefer it because I personally find it much easier to read

red031000 avatar Aug 20 '23 10:08 red031000

I prefer style B.

AsparagusEduardo avatar Aug 21 '23 14:08 AsparagusEduardo

Style A reinforces the fact that the case labels have no scoping significance and have about the same effect as a goto label. It's also wasteful to add an indent level when unnecessary.

mid-kid avatar Aug 21 '23 15:08 mid-kid

I abstain from this discussion.

luckytyphlosion avatar Aug 21 '23 17:08 luckytyphlosion

There is actually a third variant C: case labels indented by 1, but case contents not indented.

switch (state) {
    case 0:
    // ...
    case 1:
    // ...
}

I only mention this because this is the default in Notepad++. With that, I want to mention that editors are inconsistent as to which style they automatically indent to:

Editor Default behaviour Toggleable
Notepad++ Variant C Unknown (Discussion 1, Discussion 2)
Eclipse (yes this isn't C I know) Variant A Yes, see image below
Visual Studio Code Variant B Yes? (Issue which proves its existence in vscode-cpptools extension)

For eclispe: image

luckytyphlosion avatar Aug 21 '23 18:08 luckytyphlosion

There is actually a third variant C: case labels indented by 1, but case contents not indented.

switch (state) {
    case 0:
    // ...
    case 1:
    // ...
}

I only mention this because this is the default in Notepad++. With that, I want to mention that editors are inconsistent as to which style they automatically indent to:

Editor Default behaviour Toggleable Notepad++ Variant C Unknown (Discussion 1, Discussion 2) Eclipse (yes this isn't C I know) Variant A Yes, see image below Visual Studio Code Variant B Yes? (Issue which proves its existence in vscode-cpptools extension) For eclispe: image

objectively bad

red031000 avatar Aug 21 '23 18:08 red031000

There is actually a third variant C: case labels indented by 1, but case contents not indented.

It's like the worst of both worlds

AsparagusEduardo avatar Aug 22 '23 01:08 AsparagusEduardo

I prefer style B.

tgsm avatar Aug 27 '23 23:08 tgsm

This issue has had no activity for 60 days and will be marked stale. If there is no further activity, it will be closed in 30 days.

github-actions[bot] avatar Aug 12 '24 00:08 github-actions[bot]