Return to Answer

###Formatting

###Delegates

###Computed Properties

###if true then true else false

###Optional Safety

Formatting

Delegates

Computed Properties

if true then true else false

Optional Safety

It's really unfortunate that being safe with optionals is an optional thing at all. It seems mostly a hold-over from the short period of time after Swift was introduced and before Objective-C's nullability annotations.

It's really unfortunate that being safe with optionals is an optional thing at all. It seems mostly a hold-over from the short period of time after Swift was introduced and before Objective-C's nullability annotations.

###if then true else false

Here, we see another issue with overuse of vertical white space, poor optional management, and a pattern that I simply don't understand: if then true else false.

cell?.buttonEnabled = (pfObject["isEnabled"] as? Bool) ?? false

###if true then true else false

Here, we see another issue with overuse of vertical white space, poor optional management, and a pattern that I simply don't understand: if true then true else false.

cell?.buttonEnabled = (pfObject["isEnabled"] as? Bool) ?? false

###Optional Safety

It's really unfortunate that being safe with optionals is an optional thing at all. It seems mostly a hold-over from the short period of time after Swift was introduced and before Objective-C's nullability annotations .

Despite this, we should still strive to always be as safe as possible with our optionals. Force-unwrapping nil is always a crash.

if(parseObject != nil) {
 if var votes: Int? = parseObject!.objectForKey("votes") as? Int {
 votes!++
 parseObject!.setObject(votes!, forKey: "votes")
 parseObject!.saveInBackground()
 print(votes!)
 }
 delegate?.buttonClicked(self)
 loveOutlet.enabled = false
 }

Here, we have not been safe with our optionals. And the worst part here is that there's not even an imaginably good reason for this lack of safety--we're checking for nil in an if statement and only force-unwrapping within that block.

The most dangerous problem with this is that it's not thread safe. If thread 1 checks that foo is non-nil, finds it is not, then enters the if block, then thread 2 sets foo (an instance variable) to nil, then back on thread 1, we're in the if block and force unwrap, we've now crashed (despite having checked against nil). This is why we should prefer the if let construct.

if let pObj = parseObject {
 // safely use pObj as a constant, unwrapped, thread-safe, locally-scoped version of parseObject
}

The next line is particularly strange. It's a clear indication that we're aware of the if let/if var syntax, but we've used it in a mysterious and strange way.

(For the record, I absolutely never want to see votes!++ in my code base, and if a developer on my project checked that code in, I'd be looking to move that dev to a different platform. It's horrendous...)

You've used if var to properly check for nil... but then you specify the type as an optional which still has to be unwrapped, despite having already checked against nil. If we simply don't specify the type, votes would be a non-optional that doesn't need to be force unwrapped.

We should also generally prefer let variables to var variables. In fact, we should default to let and only change it to var when we absolutely must modify the value. I don't think votes is something we absolutely must modify.

Finally, keep in mind we can unwrap multiple values in a single if let statement. I'd prefer to see this code snippet as something more like this:

if let 
 pObj = parseObject, 
 votes = pObj.objectForKey("votes") as? Int {
 pObj.setObject(votes + 1, forKey: "votes")
 pObj.saveInBackground()
}
delegate?.buttonClicked(self)
loveOutlet.enabled = false

(The actual logic for when to call buttonClicked() and set the outlet disabled may be slightly different, but I wanted to provide you with this example.)