Updating number of votes with a "Like" button

I have a "Like" button in my iOS app that updates the votes every time it's tapped. Just like Facebook, Instagram, ProductHunt, etc.

I have a custom cell where that IBAction and IBOutlet is declared and that's where I'm making the query to the Parse db to update the UI (initially I'm querying the vote values from the other file from the cellForRowAtIndexPath.) In the custom cell file, I also created a delegate method, with a Protocol type that I'm accessing in my TableViewController in the cellForRowAtIndexPath.

I'm a bit of a newbie and studying the MVC data structure (assuming that's the biggest issue here), so please be easy on me!

Here is the code in the custom cell:

protocol CellButtonDelegate {

    func buttonClicked(cell : PFTableViewCell)

}
var delegate : CellButtonDelegate?

class FeedTableViewCell: PFTableViewCell {

    var delegate : CellButtonDelegate?

    var parseObject:PFObject?

    internal var buttonEnabled : Bool? {

        get {

            return loveOutlet.enabled

        } set {

            loveOutlet.enabled = newValue!
        }
    }

 @IBAction func loveButton(sender: AnyObject) {

        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 is the code in the cellForRowAtIndexPath:

```
override func tableView(tableView: UITableView, cellForRowAtIndexPath indexPath: NSIndexPath, object: PFObject?) -> PFTableViewCell? {

var cell: FeedTableViewCell? = tableView.dequeueReusableCellWithIdentifier(cellIdentifier) as? FeedTableViewCell

if(cell == nil) {

cell = NSBundle.mainBundle().loadNibNamed("FeedTableViewCell", owner: self, options: nil)[0] as? Fee

Formatting

On the whole, there are several indentation problems through out your code. Here is made the most obvious example:

if let pfObject = objectArray?[indexPath.row] {
                     cell?.parseObject = object 
var votes: Int? = pfObject["votes"] as? Int

The second line has two extra levels of indentation over what it should have. The third line needs three more indentation levels to be in the correct place.

Xcode will pretty much put every single line of code exactly where it belongs. All you have to do to get your indentation correct is simply not fight Xcode. Even if you disagree with Xcode about how something should be indented, it's usually a lot easier to just do it Xcode's way because everyone else tends to give in to Xcode... and it's a lot less effort. (For a personal example, I tend to feel like case statements within a switch should be indented one level from the switch statement, but Xcode disagrees... and it's a lot less work for me to just let Xcode have it its way.)

Delegates

var delegate : CellButtonDelegate?

Delegates should always be weak. Without making a delegate weak, you're almost always setting up some sort of a retain cycle. But what's more important, people expect for delegates to be marked as weak. If you want to replicate the typical "delegate" pattern, but for some reason need the default strong reference, then you need to call your property by something other than "delegate".

Computed Properties

I have several comments about this property:

internal var buttonEnabled : Bool? {

    get {

        return loveOutlet.enabled

    } set {

        loveOutlet.enabled = newValue!
    }
}

First of all, internal is the default access level. As you marked nothing else as public or private, everything in your code actually has this internal access level. Above all else, we should prefer consistency. If you're going to mark internal things with the internal keyword, you're now going to need to mark every property, method, extension, protocol, class, struct, enum, and function (and whatever else I forgot) with its access modifier. Anything else is inconsistent. But marking them all is too verbose. We all know (or should know) that internal is the default level, and if we mark all the internal things as internal, it just becomes an unconstructive cluttered keyword. I would only ever recommend marking something as internal in a class where basically everything else is already marked as public or private. In that case, internal indicates "No, I didn't forget to mark this one--yes, it really is internal, I promise."

Next, there's a whole lot of white space in this. In fact, you consistently have a ton of vertical white space throughout the entirety of your code. Let's use white space more constructively, shall we? Every single line of this property is all associated to this property. By eliminating the blank lines in the middle, it helps the eye associate all of these lines together visually (in a way far more immediately obvious than the curly braces do).

Finally, let's talk about optionals... Your loveOutlet doesn't appear to be defined in this file, so I don't know if it is an optional or if it is a non-optional but the enabled property is optional. I don't know what has led you to marking this buttonEnabled property as a Bool? rather than a non-optional Bool, but based on the force-unwrapping in your setter, it seems clear that this should probably be a non-optional. We should allow the user to pass nil for the buttonEnabled property--how can that even make sense? It clearly doesn't make sense, because if they ever do, we simply crash. This is supremely bad design. Swift gives you the power to clearly dictate which properties can and cannot be set to nil. What you've done here is created a property that can be set to true, false, or "crash my app please".

So more realistically, this property should look about like this:

var buttonEnabled: Bool {
    get {
        return loveOutlet.enabled
    } 
    set {
        loveOutlet.enabled = newValue
    }
}

If loveOutlet is optional (in this case, for it to compile, it'd have to be an implicitly unwrapped optional), or its enabled property is optional, then we can go for one of these two options in the get:

loveOutlet?.enabled ?? false

or

loveOutlet.enabled ?? false

if true then true else false

let isEnabled = pfObject["isEnabled"] as! Int
if isEnabled == 1 {

    cell?.buttonEnabled = true

} else {

    cell?.buttonEnabled = 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.

This as! Int can cause an app crash in multiple ways. First, the value can simply not exist for the key. Second, the value could exist, but not be convertible to an `In

StackExchange Code Review Q#112124, answer score: 6