Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style: Indent level in switch blocks #227

Open
Tracked by #220
PikalaxALT opened this issue Aug 19, 2023 · 9 comments
Open
Tracked by #220

Style: Indent level in switch blocks #227

PikalaxALT opened this issue Aug 19, 2023 · 9 comments

Comments

@PikalaxALT
Copy link
Collaborator

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:
        // ...
}
@adrienntindall
Copy link
Collaborator

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.

@red031000
Copy link
Member

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

@AsparagusEduardo
Copy link
Contributor

I prefer style B.

@mid-kid
Copy link
Member

mid-kid commented Aug 21, 2023

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.

@luckytyphlosion
Copy link
Member

I abstain from this discussion.

@luckytyphlosion
Copy link
Member

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

@red031000
Copy link
Member

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

@AsparagusEduardo
Copy link
Contributor

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

@tgsm
Copy link
Collaborator

tgsm commented Aug 27, 2023

I prefer style B.

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

No branches or pull requests

7 participants