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

Don't use ternary as an alternative for if else #2358

Closed
Igor-Palaguta opened this issue Aug 21, 2018 · 2 comments · Fixed by #3085
Closed

Don't use ternary as an alternative for if else #2358

Igor-Palaguta opened this issue Aug 21, 2018 · 2 comments · Fixed by #3085

Comments

@Igor-Palaguta
Copy link

Rule Request

  1. Why should this rule be added? Share links to existing discussion about what
    the community thinks about this.

Ternary operator should be used just on right side of assignment or in return statement. And should not be used as an alternative for if/else. As it is very unusual and harder to read than usual if/else
https://softwareengineering.stackexchange.com/a/294560

  1. Provide several examples of what would and wouldn't trigger violations.
// NOT TRIGGERED
let result = success ? "Success" : "Fail"

if success {
    askQuestion()
} else {
    exit()
}

var price: Double {
    return hasDiscount ? initialPrice - discount : initialPrice
}

// TRIGGERED
success ? askQuestion() : exit()

  1. Should the rule be configurable, if so what parameters should be configurable?

No parameters expected

  1. Should the rule be opt-in or enabled by default? Why?

It can be enabled by default

@drrost
Copy link

drrost commented Dec 4, 2018

// TRIGGERED
success ? askQuestion() : exit()

I'd suggest not allow to call functions in the 2nd and 3rd part that return void.

@drrost
Copy link

drrost commented Dec 4, 2018

  1. Should the rule be opt-in or enabled by default? Why?

It can be enabled by default

And I'd make this rule disabled by default 'cause it's quite exotic behavior for C/Objective-C/Swift and many people may want to use it as they used to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants