Style: Handling two optionals: which way is better?

Using Swift
1

For handling two optional values, which way is better?

        switch (self.region, timeZone.abbreviation(for: selection.wrappedValue)) {
        case let (.some(region), .some(abbreviation)):
            return "\(region) (\(abbreviation))"
        case let (.some(region), _):
            return region
        case let (_, .some(abbreviation)):
            return "(\(abbreviation))"
        default:
            return " "
        }

        if let region = self.region, let abbreviation = timeZone.abbreviation(for: selection.wrappedValue) {
            return "\(region) (\(abbreviation))"
        } else if let region = self.region {
            return region
        } else if let abbreviation = timeZone.abbreviation(for: selection.wrappedValue) {
            return "(\(abbreviation))"
        } else {
            return " "
        }
1 Like
2

Better in what way?

Style? You have to decide yourself, because there is no official style guide.

How easy it is to make mistakes? Switch is great at making sure you write all the cases, but you're using default which bypasses that safety feature. Tie.

How fast it is? Compiler explorer shows they generate similar amount of assembly (Compiler Explorer), but in 2 abbreviation(for:) is called twice. 1 wins here (but if you care about performance, you should benchmark it!)

2 Likes
3

The first one is better but I think

let emptyString = ""
return ("(region ?? emptyString) (abbreviation ?? emptyString)")

With the nil-coalescing operator, might be cleaner.

—Dan

2 Likes
4

I would do 1 but switching out the wildcards (_) and default with explicit nils

Said that, it looks like it could be expressed in a more declarative way, but the fact that the default case returns ” “ confuses me

5

I have to specifically return a one space string when both are nil...because the result of this is shown on screen with SwiftUI.Text. If I return an empty string "", the Text("") is nothing which mess up my layout. So I give it one space string for the Text to take up the room.

1 Like
6

Then I would write something like

let base = [
    region,
    abbreviation.map({ "(\($0))" }),
].compactMap({ $0 })
  .joined(separator: " ")
// Avoid returning the empty String to make sure a Text always takes some space
return base.isEmpty ? " " : base

Edit: forgot the joined

1 Like
7

I understand this part:

        switch (self.region, timeZone.abbreviation(for: selection.wrappedValue)) {
        case let (.some(region), .some(abbreviation)):
            return "\(region) (\(abbreviation))"
        case let (.some(region), nil):
            return region
        case let (nil, .some(abbreviation)):
            return "(\(abbreviation))"
        default:
            return " "
        }

I don't understand this part...can you explain?

8

You can write case (nil, nil): instead of default so that you explicitly check all possible cases (and switch compilation protects you in ensuring that you did get all cases)

2 Likes
9

:+1:Think more declarative, I like this!
:pray:

1 Like
10

Note you can sugar this as case let (region?, abbreviation?):.

(as well as case let (_?, abbreviation?) to match only non-nil values where you can't care about the payload)

5 Likes
11

A switch seems overkill here.

let regionString = self.region ?? ""
let abbreviationString = timeZone.abbreviation(for: selection.wrappedValue).map { "(\($0)" } ?? ""

return regionString + " " + abbreviationString
12

This solution leaves a trailing whitespace in the case of one nil between the two

13

From the code in OP, that seems to be the requirement: when both are nil the function returns " ".

EDIT: you're right, you mean a trailing whitespace. Let me fix...

14

When both are nil, yes. When only one is, it's only the part

15

fixed:

let region = self.region
let abbreviation = timeZone.abbreviation(for: selection.wrappedValue)

let regionString = region ?? ""
let spaceString = (region == nil) == (abbreviation == nil) ? " " : ""
let abbreviationString = abbreviation.map { "(\($0)" } ?? ""

return regionString + spaceString + abbreviationString
2 Likes
16

Riffing on yours, this preserves the space between:

return "\(region ?? "") \(abbreviation ?? "")".trimmingCharacters(in: [" "])
2 Likes