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

Golden Path #63

Closed
JRG-Developer opened this Issue Nov 4, 2014 · 17 comments

Comments

Projects
None yet
7 participants
@JRG-Developer
Member

JRG-Developer commented Nov 4, 2014

To steal from our Objective-C style guide, I propose the inclusion of the Golden Path rule in this guide:

Golden Path

When coding with conditionals, the left hand margin of the code should be the "golden" or "happy" path. That is, don't nest if statements. Multiple return statements are OK.

Preferred:

func someMethod() {

  if someBoolValue == false {
    return
  }

  // Do something important
}

Not Preferred:

func someMethod() {

  if someBoolValue == true {
    // Do something important
  }
}
@gregheo

This comment has been minimized.

Show comment
Hide comment
@gregheo

gregheo Dec 3, 2014

Member

I would agree here but what about optional binding? Sometimes there are a big mess of if let cases stacked up with the "happy" code way down in all the curly braces.

I suppose we could replace that with many checks for nil, but that seems against the Swift way.

Member

gregheo commented Dec 3, 2014

I would agree here but what about optional binding? Sometimes there are a big mess of if let cases stacked up with the "happy" code way down in all the curly braces.

I suppose we could replace that with many checks for nil, but that seems against the Swift way.

@cwagdev

This comment has been minimized.

Show comment
Hide comment
@cwagdev

cwagdev Dec 3, 2014

Checking for nil is definitely going against swift in my opinion. Then when you're done you're left to use forced unwrapping which feels even worse. 


Chris

On Tue, Dec 2, 2014 at 6:54 PM, Greg Heo notifications@github.com wrote:

I would agree here but what about optional binding? Sometimes there are a big mess of if let cases stacked up with the "happy" code way down in all the curly braces.

I suppose we could replace that with many checks for nil, but that seems against the Swift way.

Reply to this email directly or view it on GitHub:
#63 (comment)

cwagdev commented Dec 3, 2014

Checking for nil is definitely going against swift in my opinion. Then when you're done you're left to use forced unwrapping which feels even worse. 


Chris

On Tue, Dec 2, 2014 at 6:54 PM, Greg Heo notifications@github.com wrote:

I would agree here but what about optional binding? Sometimes there are a big mess of if let cases stacked up with the "happy" code way down in all the curly braces.

I suppose we could replace that with many checks for nil, but that seems against the Swift way.

Reply to this email directly or view it on GitHub:
#63 (comment)

@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Dec 4, 2014

Member

I think the scenario we should discourage is nesting a big mess of if let statements.

E.g.

if let object1= optional1 {
  if let object2 = optional2 {
     if let object3 = optional3 {
        // important code is nested deep inside here...
     }
   }
} else {
  // handle missing values
}

Seems bad.

Instead, I propose this is cleaner:

if optional1 == nil || optional2 == nil || optional3 == nil {
  // handle missing values
  return;
}

let object1 = optional1!
let object2 = optional2!
let object3 = optional3!

// important code is aligned with the left
Member

JRG-Developer commented Dec 4, 2014

I think the scenario we should discourage is nesting a big mess of if let statements.

E.g.

if let object1= optional1 {
  if let object2 = optional2 {
     if let object3 = optional3 {
        // important code is nested deep inside here...
     }
   }
} else {
  // handle missing values
}

Seems bad.

Instead, I propose this is cleaner:

if optional1 == nil || optional2 == nil || optional3 == nil {
  // handle missing values
  return;
}

let object1 = optional1!
let object2 = optional2!
let object3 = optional3!

// important code is aligned with the left
@rwenderlich

This comment has been minimized.

Show comment
Hide comment
@rwenderlich

rwenderlich Dec 4, 2014

Member

Hm - I worry about that second approach - it seems more error-prone than nested if lets. What if someone adds a let object4 = optional4! but forgets to add it in the if statement? Also I think ! should be avoided as much as possible, we shouldn't be encouraging it...

Member

rwenderlich commented Dec 4, 2014

Hm - I worry about that second approach - it seems more error-prone than nested if lets. What if someone adds a let object4 = optional4! but forgets to add it in the if statement? Also I think ! should be avoided as much as possible, we shouldn't be encouraging it...

@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Dec 4, 2014

Member

If they added another let object4 = optional4! without adding it to the if statement, you would hope that the app would crash quickly, but I can see how this may persist as a silent bug if the input is present most of the time...

Maybe there isn't a good solution to this issue, and it should be addressed on a case by case basis?

For example, start by questioning, "Why do I have four optionals here...?"

Unless others have a better suggestion on how to address this, perhaps this should simply be closed...?

Member

JRG-Developer commented Dec 4, 2014

If they added another let object4 = optional4! without adding it to the if statement, you would hope that the app would crash quickly, but I can see how this may persist as a silent bug if the input is present most of the time...

Maybe there isn't a good solution to this issue, and it should be addressed on a case by case basis?

For example, start by questioning, "Why do I have four optionals here...?"

Unless others have a better suggestion on how to address this, perhaps this should simply be closed...?

@gregheo

This comment has been minimized.

Show comment
Hide comment
@gregheo

gregheo Dec 9, 2014

Member

I'd worry a little bit too, especially if there might be "in between" work to do, e.g.:

if let object1= optional1 {
  // do something with object1 here
  if let object2 = optional2 {
     if let object3 = optional3 {
       // do something with objects 1,2,3
     }
   }

I guess you could split that up into two if blocks, one for just object1 and another for all of 1,2,3.

I feel like Swift is going to get multiple bindings some day, like this:

if let object1 = optional1 && object2 = optional2 {
}

In that case, I'd have to argue against the usual golden path and say it's an established Swift idiom to have the "happy path" inside the conditional.

Member

gregheo commented Dec 9, 2014

I'd worry a little bit too, especially if there might be "in between" work to do, e.g.:

if let object1= optional1 {
  // do something with object1 here
  if let object2 = optional2 {
     if let object3 = optional3 {
       // do something with objects 1,2,3
     }
   }

I guess you could split that up into two if blocks, one for just object1 and another for all of 1,2,3.

I feel like Swift is going to get multiple bindings some day, like this:

if let object1 = optional1 && object2 = optional2 {
}

In that case, I'd have to argue against the usual golden path and say it's an established Swift idiom to have the "happy path" inside the conditional.

@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Dec 11, 2014

Member

Closing this issue as the group seems to agree that the Golden Path has undesirable consequences in Swift.

As @gregheo mentions, hopefully Swift will have multiple let bindings included one day. When such day comes, perhaps such can be recommended to be added to the guide.

:]

Member

JRG-Developer commented Dec 11, 2014

Closing this issue as the group seems to agree that the Golden Path has undesirable consequences in Swift.

As @gregheo mentions, hopefully Swift will have multiple let bindings included one day. When such day comes, perhaps such can be recommended to be added to the guide.

:]

@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Dec 29, 2015

Member

With the introduction of guard in Swift 2.0, I think we should consider adding the Golden Path to our Swift guide.

I think many agree this is a good idea in Swift now and are already following it in their writing.

Member

JRG-Developer commented Dec 29, 2015

With the introduction of guard in Swift 2.0, I think we should consider adding the Golden Path to our Swift guide.

I think many agree this is a good idea in Swift now and are already following it in their writing.

@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Dec 29, 2015

Member

Also, multiple guard / let bindings are now allowed by putting a comma between them. :]

Member

JRG-Developer commented Dec 29, 2015

Also, multiple guard / let bindings are now allowed by putting a comma between them. :]

@lukewar

This comment has been minimized.

Show comment
Hide comment
@lukewar

lukewar Dec 30, 2015

I agree. Example from above in Swift 2.0 should look more like this:

guard let optional1 = optional1, let optional2 = optional2, let optional3 = optional3 else {
    // handle missing values
    return;
}

// important code is aligned with the left

lukewar commented Dec 30, 2015

I agree. Example from above in Swift 2.0 should look more like this:

guard let optional1 = optional1, let optional2 = optional2, let optional3 = optional3 else {
    // handle missing values
    return;
}

// important code is aligned with the left
@rayfix

This comment has been minimized.

Show comment
Hide comment
@rayfix

rayfix Apr 7, 2016

Contributor

Approved. I will be adding a section about golden path and guard in the style guide. @lukewar I like your example but ... that semicolon ... 😆

Contributor

rayfix commented Apr 7, 2016

Approved. I will be adding a section about golden path and guard in the style guide. @lukewar I like your example but ... that semicolon ... 😆

@lukewar

This comment has been minimized.

Show comment
Hide comment
@lukewar

lukewar Apr 7, 2016

lol, jumping between Swift and ObjC is harder than one would think :P

lukewar commented Apr 7, 2016

lol, jumping between Swift and ObjC is harder than one would think :P

@RobertGummesson

This comment has been minimized.

Show comment
Hide comment
@RobertGummesson

RobertGummesson Apr 7, 2016

Contributor

How about dropping the let duplicates from that example? You guys think they add clarity?

guard let optional1 = optional1, optional2 = optional2, optional3 = optional3....

Contributor

RobertGummesson commented Apr 7, 2016

How about dropping the let duplicates from that example? You guys think they add clarity?

guard let optional1 = optional1, optional2 = optional2, optional3 = optional3....

@rayfix rayfix self-assigned this Apr 7, 2016

@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Apr 7, 2016

Member

I'm against dropping the let duplicates, or at least, making a hard rule there shouldn't be any duplicates.

Reasoning:

(1) If you include a where clause, you must specify let again. For example, this is a problem:

Not Allowed:

func doSomethingWithOptionalNumber(number1: Int?, number2: Int?) {

  guard let number1 = number1 where number1 > number2,
    number2 = number2 else {   //  COMPILER ERROR
      print("Number criteria not met!")
      return
  }

  // ... here be magic...
}

(2) If you have a mix of var and let, it can be unclear what your intentions are if you omit the let/var specifier:

Potentially Unclear:

guard let number1 = number1,
  var number2 = number2,
  number3 = number3 else { // Did you really mean for this to be `var` or actually `let`?
    return
}

I'd propose:

  • There should always be a let/ var, even if redundant.
  • Or, the decision should be left up to the tutorial author, depending on context.
Member

JRG-Developer commented Apr 7, 2016

I'm against dropping the let duplicates, or at least, making a hard rule there shouldn't be any duplicates.

Reasoning:

(1) If you include a where clause, you must specify let again. For example, this is a problem:

Not Allowed:

func doSomethingWithOptionalNumber(number1: Int?, number2: Int?) {

  guard let number1 = number1 where number1 > number2,
    number2 = number2 else {   //  COMPILER ERROR
      print("Number criteria not met!")
      return
  }

  // ... here be magic...
}

(2) If you have a mix of var and let, it can be unclear what your intentions are if you omit the let/var specifier:

Potentially Unclear:

guard let number1 = number1,
  var number2 = number2,
  number3 = number3 else { // Did you really mean for this to be `var` or actually `let`?
    return
}

I'd propose:

  • There should always be a let/ var, even if redundant.
  • Or, the decision should be left up to the tutorial author, depending on context.
@rayfix

This comment has been minimized.

Show comment
Hide comment
@rayfix

rayfix Apr 7, 2016

Contributor

Use of var is deprecated (and will be an error in Swift 3) so that argument is out. Also, if you order correctly you can omit the let with where.

func doSomethingWithOptionalNumber(number1: Int?, number2: Int?) {

  guard let number1 = number1, number2 = number2 where number1 > number2
    else {   //  COMPILER ERROR
      print("Number criteria not met!")
      return
  }

  // ... here be magic...
}

That said, I understand that a style guide shouldn't be a straight jacket. While I may elide the let from the example, we don't have to necessarily rule on it.

Contributor

rayfix commented Apr 7, 2016

Use of var is deprecated (and will be an error in Swift 3) so that argument is out. Also, if you order correctly you can omit the let with where.

func doSomethingWithOptionalNumber(number1: Int?, number2: Int?) {

  guard let number1 = number1, number2 = number2 where number1 > number2
    else {   //  COMPILER ERROR
      print("Number criteria not met!")
      return
  }

  // ... here be magic...
}

That said, I understand that a style guide shouldn't be a straight jacket. While I may elide the let from the example, we don't have to necessarily rule on it.

@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Apr 7, 2016

Member

@rayfix Admittedly, these were contrived examples. 😉

However, you definitely won't always be able to reorder to get rid of the necessity for the second let.

Regarding,

Use of var is deprecated

You can only have let variables now?! That's definitely quite a shift to functional programming in Swift!

😱 😱 🙀

Jokes aside though, only var method parameters are deprecated. You can definitely have var in a guard statement. Here's an example:

func addDefaultHeadersTo(headers: [String: String]?) -> [String: String] {

  let defaultHeaders = ["AppVersion": "1.0.42",
                        "Who's Awesome?": "Ray Fix"]

  guard var headers = headers else {
    return defaultHeaders
  }

  for (key, value) in defaultHeaders {
    headers[key] = value
  }

  return headers
}
Member

JRG-Developer commented Apr 7, 2016

@rayfix Admittedly, these were contrived examples. 😉

However, you definitely won't always be able to reorder to get rid of the necessity for the second let.

Regarding,

Use of var is deprecated

You can only have let variables now?! That's definitely quite a shift to functional programming in Swift!

😱 😱 🙀

Jokes aside though, only var method parameters are deprecated. You can definitely have var in a guard statement. Here's an example:

func addDefaultHeadersTo(headers: [String: String]?) -> [String: String] {

  let defaultHeaders = ["AppVersion": "1.0.42",
                        "Who's Awesome?": "Ray Fix"]

  guard var headers = headers else {
    return defaultHeaders
  }

  for (key, value) in defaultHeaders {
    headers[key] = value
  }

  return headers
}
@JRG-Developer

This comment has been minimized.

Show comment
Hide comment
@JRG-Developer

JRG-Developer Apr 8, 2016

Member

That said, I understand that a style guide shouldn't be a straight jacket. While I may elide the let from the example, we don't have to necessarily rule on it.

👍

Member

JRG-Developer commented Apr 8, 2016

That said, I understand that a style guide shouldn't be a straight jacket. While I may elide the let from the example, we don't have to necessarily rule on it.

👍

@rayfix rayfix added this to the Update April 2016 milestone Apr 8, 2016

@rayfix rayfix added Review and removed Review labels Apr 13, 2016

@rayfix rayfix closed this Apr 13, 2016

@mrdekk mrdekk referenced this issue Nov 11, 2016

Merged

Basic UI #1

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