iOS MQTT client app base view controller

Question

I've been working on an iOS IOT client app which uses MQTT. After some reading I decided to go with CocoaMQTT.

I have only been programming in swift/iOS for about 2 weeks now, so I thought I should have my base code reviewed here before continuing any further.

The idea is to have most of the MQTT connection handling code in a MQTTViewController class, which itself subclasses UIViewController. This MQTTViewController class would conform to the CocoaMQTTDelegate protocol, providing overridable stubs for all delegate functions.

All other view controllers will subclass the MQTTViewController.

I also want to pass my CocoaMQTT instance and some state variables back-and-forth, so I don't need to connect each time. Currently, I'm doing this by providing a helper function pushMQTTViewControllerWithIdentifier() for push and using MQTTPassBackDelegate protocol function passBackMQTTData() for pop.

Here's my code:

import UIKit
import CocoaMQTT


class MQTTViewController: UIViewController, CocoaMQTTDelegate, MQTTPassBackDelegate {

    // MARK: Internal classes
    class MQTTState {
        var connected = false
        var loggedIn = false
    }


    // MARK: Properties
    var mqtt: CocoaMQTT? {
        didSet {
            if mqtt == nil {
                state.connected = false
                state.loggedIn = false
                if oldValue?.connState == .CONNECTED {
                    oldValue?.disconnect()
                }
            }
        }
    }

    var state = MQTTState()
    var username: String?
    var password: String?

    weak var delegate: MQTTPassBackDelegate? = nil


    // MARK: Lifecycle
    override func viewWillAppear(animated: Bool) {
        super.viewWillAppear(animated)

        guard mqtt != nil else {
            mqttInit()
            return
        }
    }

    override func viewWillDisappear(animated: Bool) {
        super.viewWillDisappear(animated)

        if self.isMovingFromParentViewController() == true {
            delegate?.passBackMQTTData(mqtt, state: state, username: username, password: password)
        }
    }


    // MARK: CocoaMQTTDelegate functions
    func mqtt(mqtt: CocoaMQTT, didConnect host: String, port: Int) {
        dprint("didConnect \(host):\(port)")
    }

    func mqtt(mqtt: CocoaMQTT, didConnectAck ack: CocoaMQTTConnAck) {
        dprint("didConnectAck \(ack.rawValue)")
    }

    func mqtt(mqtt: CocoaMQTT, didPublishMessage message: CocoaMQTTMessage, id: UInt16) {
        dprint("didPublishMessage with message: \(message.string)")
    }

    func mqtt(mqtt: CocoaMQTT, didPublishAck id: UInt16) {
        dprint("didPublishAck with id: \(id)")
    }

    func mqtt(mqtt: CocoaMQTT, didReceiveMessage message: CocoaMQTTMessage, id: UInt16 ) {
        guard message.string != nil else {
            dprint("didReceivedMessage: NOT STRING with id \(id)")
            return
        }
        dprint("didReceivedMessage: \(message.string) with id \(id)")
    }

    func mqtt(mqtt: CocoaMQTT, didSubscribeTopic topic: String) {
        dprint("didSubscribeTopic to \(topic)")
    }

    func mqtt(mqtt: CocoaMQTT, didUnsubscribeTopic topic: String) {
        dprint("didUnsubscribeTopic to \(topic)")
    }

    func mqttDidPing(mqtt: CocoaMQTT) {
        dprint("didPing")
    }

    func mqttDidReceivePong(mqtt: CocoaMQTT) {
        dprint("didReceivePong")
    }

    func mqttDidDisconnect(mqtt: CocoaMQTT, withError err: NSError?) {
        dprint("mqttDidDisconnect")
    }


    // MARK: MQTTPassBackDelegate functions
    func passBackMQTTData(mqtt: CocoaMQTT?, state: AnyObject, username: String?, password: String?) {
        dprint("passBackMQTTData")
        self.mqtt = mqtt
        self.state = state as! MQTTState
        self.username = username
        self.password = password
    }


    // MARK: Helpers
    final func pushMQTTViewControllerWithIdentifier(identifier: String) {
        let vc = storyboard?.instantiateViewControllerWithIdentifier(identifier) as! MQTTViewController
        vc.mqtt = mqtt
        vc.state = state
        vc.username = username
        vc.password = password
        vc.delegate = self
        navigationController!.pushViewController(vc, animated: true)
    }

    final func mqttInit(clientIdPid: String = "ACC_MQTT-" + String(NSProcessInfo().processIdentifier)) {
        guard mqtt == nil else {
            return      // Prevent reinit
        }
        mqtt = CocoaMQTT(clientId: clientIdPid, host: ACC_MQTT_SERVER, port: UInt16(ACC_MQTT_PORT))
    }

    final func mqttSetup(username: String, password: String) {
        print("Attempting to connect using \(username):\(password)")
        if let mqtt = mqtt {
            mqtt.username = username
            mqtt.password = password
            mqtt.cleanSess = false
            mqtt.keepAlive = 90
            mqtt.delegate = self
        }
    }

}


protocol MQTTPassBackDelegate: class {

    func passBackMQTTData(mqtt: CocoaMQTT?, state: AnyObject, username: String?, password: String?)

}

1. Is this the right way going forward?
2. Will passing data between view controllers this way result in strong reference cycles?

Answer 1

As I understand it, MQTT is a communications protocol, much like web sockets or HTTP... As such, I believe your approach is the wrong way to go.

The app's UI should be isolated from its communications protocol, but you have intertwined the two so pervasively that it will be impossible to change one without affecting the other.

Instead, you should separate the concerns. No file in your app should import both CocoaMQTT and UIKit. Your view controllers should not implement CocoaMQTTDelegate or MQTTPassBackDelegate. Rather you should have a separate object that implements those protocols that the view controllers communicate with using a higher level of abstraction.

Even if this separate object is a singleton instance, your architecture will be better for it, or you can go all out and pass the communications object from view controller to VC using the standard prepare for segue and unwind idioms.

To be more concrete about it, you need some sort of Server class. It's methods will detail what data can be sent to and received from the server without exposing the minutia of how that data actually gets sent and received.