Decode an alarm keypad message

Question

In my efforts to learn the Swift language, I am trying to mix in a pet project along with my learning of the basic language syntax. In essence, i'm trying to write an iOS keypad application to work with my house alarm, which I have utilizing a device from Alarm Decoder.

The device basically opens up a socket which I have successfully written the code to interact with - I will probably post that for review at some point but right now i'm focusing on decoding the messages that I get back from the device.

Based on my knowledge so far of the Swift language, I have written a function which attempts to decode the message, and in turn populate another variable with various properties. Please can you look over the code and provide feedback if I am even going about this the correct way.

I'm not expecting anyone to re-write the function for me, more just point me in the right direction if i'm off the mark or doing something silly; i'm more than happy to go and research any other way you feel appropriate. As said, i'm learning here...

import Foundation

struct AD2PIKeypadMessage {

    var displayMessage: String = ""
    var isReady: Bool = false
    var isArmedAway: Bool = false
    var isArmedHome: Bool = false
    var isChimeEnabled: Bool = false


} // struct

func formatKeypadMessage(var message: String) -> AD2PIKeypadMessage? {

    // Alarm Decoder keypad message protocol information - http://www.alarmdecoder.com/wiki/index.php/Protocol

    // keypad messages from Alarm Decoder generally look like this:
    // [01000001000---------],0e5,[f707000600e5800c0c020000],"ARMED ***AWAY***** ALL SECURE **"

    // ... or with firmware 2.2a.8+ and config bit 0x0008 set, they can look like this:
    // !KPM:[001000010000--------],010,[f70700060010808c08020000],"ARMED ***STAY** ZONE BYPASSED"

    // First check for blank strings...
    if ( message.isEmpty ) {
        return(nil)
    } // if

    // If the message is prefixed with !KPM, remove it
    if ( message.hasPrefix("!KPM:") ) {
        message = message.substringFromIndex(advance(message.startIndex, 5))
    } // if

    // Split our string into an array using the commas - there should be 4 array elements
    let messageArray = message.componentsSeparatedByString(",")
    if ( 4 == messageArray.count ) {

        // The first array element should be a string of 22 characters
        if ( 22 == count(messageArray[0]) ) {

            // ... it should also start and end with square brackets
            if messageArray[0].hasPrefix("[") && messageArray[0].hasSuffix("]") {

                // At this stage, the message is probably valid so generate a message variable so that
                // we can start populating it...

                var kpiReturnMessage: AD2PIKeypadMessage = AD2PIKeypadMessage()
                kpiReturnMessage.displayMessage = messageArray[3]  // The 4th array element is just display text and could be anything

                // Now lets split the alarm properties string up to an array so its easier for us to use
                let alarmPropertiesArray = Array(messageArray[0])

                // Set the various elements in our message variable - remember the 
                // first array element will be the opening square bracket so we ignore that

                if ( "1" == alarmPropertiesArray[1] ) {
                    kpiReturnMessage.isReady = true
                } // if

                if ( "1" == alarmPropertiesArray[2] ) {
                    kpiReturnMessage.isArmedAway = true
                } // if

                if ( "1" == alarmPropertiesArray[3] ) {
                    kpiReturnMessage.isArmedHome = true
                } // if

                if ( "1" == alarmPropertiesArray[9] ) {
                    kpiReturnMessage.isChimeEnabled = true
                } // if

                return(kpiReturnMessage)

            } else {
                return(nil)
            } // if

        } else {
            return(nil)
        } // if

    } else {
        return(nil)
    } // if

} // func




if let kpm: AD2PIKeypadMessage? = formatKeypadMessage("[01000001000---------],0e5,[f707000600e5800c0c020000],\"ARMED ***AWAY***** ALL SECURE **\"") {

    println("Valid Keypad Message")

} // if

Answer 1

One thing I'm noticing is that you have a lot of "magic values". Things like strings and numbers that need to be explained by a comment. I'd create the following constants:

// Some useful string constants we'll need
let kMessagePrefix = "!KPM:"
let kTrue = "1"
// Important numerical constants
let kValidMessageCount = 4
let kValidMessageLength = 22
let kIsReadyIndex = 1
let kIsArmedAwayIndex = 2
let kIsArmedHomeIndex = 3
let kIsChimeEnabledIndex = 9

You can then do things like this:

if (message.hasPrefix(kMessagePrefix)) {

This way the code describes what you're looking for. It's not just some random string. It's the prefix of all messages.

Next, since you have the prefix as a string, use that string to get the rest of the message like this:

    message = message.substringFromIndex(advance(message.startIndex, count(kMessagePrefix)))

You can remove all of the else cases. In the case where the message is valid and you can get the data out of it, you return a valid record. In all other cases, it falls through. So you can reformat your code as:

if (kValidMessageCount == messageArray.count ) {
    // ... check all the other stuff
    return (kpiReturnMessage)
}

return (nil)

It makes it much easier to read.

Also, you use messageArray[0] a lot. Why not assign it to a variable (or better a constant since you aren't changing it)?

let keypadMessage = messageArray[0];
if (kValidMessageLength == count(keypadMessage)) {
        // ... it should also start and end with square brackets
        if keypadMessage.hasPrefix("[") && keypadMessage.hasSuffix("]") {

            // At this stage, the message is probably valid so generate a message variable so that
            // we can start populating it...

            var kpiReturnMessage: AD2PIKeypadMessage = AD2PIKeypadMessage()
            kpiReturnMessage.displayMessage = messageArray[3]  // The 4th array element is just display text and could be anything

            // Now lets split the alarm properties string up to an array so its easier for us to use
            let alarmPropertiesArray = Array(keypadMessage)

            // Set the various elements in our message variable - remember the 
            // first array element will be the opening square bracket so we ignore that

            if ( kTrue == alarmPropertiesArray[kIsReadyIndex] ) {
                kpiReturnMessage.isReady = true
            } // if

            if ( kTrue == alarmPropertiesArray[kIsArmedAwayIndex] ) {
                kpiReturnMessage.isArmedAway = true
            } // if

            if ( kTrue == alarmPropertiesArray[kIsArmedHomeIndex] ) {
                kpiReturnMessage.isArmedHome = true
            } // if

            if ( kTrue == alarmPropertiesArray[kIsChimeEnabledIndex] ) {
                kpiReturnMessage.isChimeEnabled = true
            } // if

            return(kpiReturnMessage)
    }
}

If you think you're going to check more boolean properties, then you might want to make the if statements into their own function.