Guard statement styling

[gokselkoksal]

Hi,

I am seeing a lot of inconsistent guard statements. Can we add a section for guard statements in styleguide?

Suggesstion:

One statement:

guard let name = json["name"] as? String else {
    return nil
}

Multi statement:

guard let name = json["name"] as? String,
    let coordinatesJSON = json["coordinates"] as? [String: Double],
    let latitude = coordinatesJSON["lat"],
    let longitude = coordinatesJSON["lng"],
    let mealsJSON = json["meals"] as? [String]
else {
    return nil
}

[rayfix]

I like this suggestion.... modulo 2-space 4-space indentation :]

[rayfix]

Starting to have second thoughts about this one. I have seen a lot of swift code, from libraries authors that I really admire doing guards as one liners. Any thoughts @JRG-Developer @eskerber @jawwad @gregheo ?

Possible compromise: show all of the examples using the above format but not legislate on it.

[JRG-Developer]

Here are my thoughts:

• First and foremost, make the text look good in print/web/whatever format is intended. Often, this means adding returns where you normally might not have included returns in Xcode.

• For single-statement guards, I actually prefer this:

guard let strongSelf = self else { return nil }

My reasoning is that it's not typically critical to whatever the main purpose of the method is. Sure, it's definitely required to be there, but I don't think it's worthwhile to waste three lines on simple guards like this.

This also helps to make text in print, web, etc a bit shorter, which is a plus.

• For multi-statement guards, I prefer this:

guard let name = json["name"] as? String,
  let coordinatesJSON = json["coordinates"] as? [String: Double],
  let latitude = coordinatesJSON["lat"],
  let longitude = coordinatesJSON["lng"],
  let mealsJSON = json["meals"] as? [String] else { return nil }

The difference here is keeping the else on the same line as the last unwrapping.
The reasoning is the same: this isn't the expected path, and it's not worthwhile to waste the space.

[gokselkoksal]

I borrowed the initial format from Apple but then I changed my code style too. It's reasonable to use a one-liner if you are returning instantly.

Here is the new version:

Single:

guard let name = json["name"] as? String else { return nil }

Multi:

guard let name = json["name"] as? String,
    let coordinatesJSON = json["coordinates"] as? [String: Double],
    let latitude = coordinatesJSON["lat"],
    let longitude = coordinatesJSON["lng"],
    let mealsJSON = json["meals"] as? [String]
else { return nil }

I think else deserves its own line when there are multiple lets.

[rayfix]

Having it on one line prevents you from putting a breakpoint on the failure case no?

[gokselkoksal]

@rayfix yeap, you can't do that... ¯_(ツ)_/¯

[RobertGummesson]

I’ve tried and used both of these styles and my preference ended up being the one line option too. In my opinion, having more terse and readable code outweigh any disadvantage.

I can see where people who disagree are coming from though. Other than problem @rayfix mentioned, I see two other downsides.

Sometimes they end up being relatively long, hence making them less readable. (I'm sure I could find more extreme examples):

Multiple lines:

guard let meal = Meal(rawValue: string) else {
    throw SerializationError.invalid("meals", string)
}

One line:

guard let meal = Meal(rawValue: string) else { throw SerializationError.invalid("meals", string) }

The other problem is just consistency. I would never do a one line if condition for example.

But again, my preference is still using one line. Just trying to turn a few more rocks.

[rayfix]

My inclination is that there is no hard and fast rule on this. Maybe we should be explicit about that the way we were with closure chaining.

Decisions on spacing, line breaks, and when to use named versus anonymous arguments is left to the discretion of the author.

[rayfix]

I did an informal survey of some highly starred Swift libraries including the Swift Standard Library, Almofire, Unbox, SwiftyJSON, Spring. There is no real consensus that I can find. They all look pretty good to me in their own context.

Even in the Apple JSON example from https://developer.apple.com/swift/blog/?id=37 they put the else in different places in the same function! That isn't to say it looks bad. It actually looks good to my eye.

So I think I am falling into the camp of wanting to embrace the flexibility that the language allows. I think this favors readability (clarity) at the expense of some consistency.

I could be wrong, but I don't want to hold the release on this issue. My goal is to have the document reviewed by the team on Monday so that it can be merged on Thursday. If you want to appeal I am totally cool with that! Please state your case (here) and I will forward it to the team leads to make a final decision.

Regardless, I want to thank you for bringing up this issue as it has given me better clarity and sparked some useful discussion and insights.