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

Consider alternative style for ternary operators #598

Closed
Gozala opened this issue Aug 19, 2016 · 6 comments

Comments

@Gozala
Copy link

commented Aug 19, 2016

Is there a chance I could persuade you to adopt alternative style for ternary operators:

var location =
  ( env.development
  ? 'localhost'
  : 'www.api.com'
  )

It has better readability than case where first condition is placed on the same line as assignment, especially if many ternaries are chained.

Note that this is almost accepted by standard it just does not like space after (:

var location =
  (env.development
  ? 'localhost'
  : 'www.api.com'
  )

Thanks for taking time to read & consider.

@LinusU

This comment has been minimized.

Copy link
Member

commented Aug 19, 2016

I don't think that it looks that readable, have you considered the following style?

var location = env.development
  ? 'localhost'
  : 'www.api.com'
@Gozala

This comment has been minimized.

Copy link
Author

commented Aug 19, 2016

Well proposed style is also more compatible with ternaries as arguments, while currently recommended option is not.

@feross

This comment has been minimized.

Copy link
Member

commented Aug 19, 2016

Thanks for the suggestion, but I think the current style is clearest. Also, there's not even a way to configure ESLint to allow the style you've suggested, since it's pretty unique (I've never seen it before)

@feross feross closed this Aug 19, 2016

@feross feross added the question label Aug 19, 2016

@Gozala

This comment has been minimized.

Copy link
Author

commented Aug 25, 2016

@feross mind clearing up how following examples should be formatted then ?

export const clip =
  (min:number, max:number, n:number):number =>
  ( n < min
  ? min
  : n > max
  ? max
  : n
  )

Or case with multiple branches:

const thingy =
  ( message.type === "ForFoo"
  ? ( model.kind === "Foo"
    ? doSomething(model, message.payload)
    : panic(model, message)
    )
  : message.type === "ForBar"
  ? ( model.kind === "Bar"
    ? doOtherThingk(model, message.payload)
    : panic(model, message)
    )
  : panic(model, message)
  )

Thanks

P.S.: Just trying to figure out how to reformat our current code base to standard & run into things that look little odd with standardjs

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2016

@Gozala first time I'm seeing code like you wrote it. Perhaps this valid standard syntax might be closer to liking:

const thingy = (message.type === 'ForFoo')
  ? (model.kind === 'Foo')
    ? doSomething(model, message.payload)
    : panic(model, message)
  : (message.type === 'ForBar')
    ? (model.kind === 'Bar')
      ? doOtherThingk(model, message.payload)
      : panic(model, message)
    : panic(model, message)
@dcousens

This comment has been minimized.

Copy link
Member

commented Aug 26, 2016

Multi-ternary expressions.

Mmmm.
Smells like an eslint feature request.

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
5 participants
You can’t perform that action at this time.