17
\$\begingroup\$

I've seen a lot of Objective-C implementations but I'd like to do it in Swift.

I did it like this and it seems to work just fine. Does anyone have comments and/or improvements to make?

import SystemConfiguration

class EnvData: NSObject {

    class func getSSID() -> String {

        var currentSSID = ""

        let interfaces = CNCopySupportedInterfaces()

        if interfaces != nil {

            let interfacesArray = interfaces.takeRetainedValue() as [String]

            if interfacesArray.count > 0 {

                let interfaceName = interfacesArray[0] as String

                let unsafeInterfaceData = CNCopyCurrentNetworkInfo(interfaceName)

                if unsafeInterfaceData != nil {

                    let interfaceData = unsafeInterfaceData.takeRetainedValue() as Dictionary!

                    currentSSID = interfaceData["SSID"] as String

                } else {

                    currentSSID = ""
                }

            } else {

                currentSSID = ""
            }

        } else {

            currentSSID = ""
        }

        return currentSSID
    }
}
\$\endgroup\$

2 Answers 2

16
\$\begingroup\$

The first thing that stands out to me is that you've created an entirely new class that exists purely to have this class method. This doesn't make a whole lot of sense. At a minimum, this method could just exist on its own, outside of a class, but I'll do you one better: let's write it as an extension to UIDevice, that's where it makes the most sense, and there are already other similar properties on UIDevice...

extension UIDevice {
    public var SSID: String {
        get {
            // your code
        }
    }
}

Now, getting the SSID will be similar to getting the device name, or system version, or other useful things about the device you might need to know:

let ssid = UIDevice.currentDevice().SSID

Now then, on to the actual meat of your code...

For starters, I don't know much about getting SSIDs. Is it at all possible that you wouldn't be able to fetch the SSID? What is the likelihood of actually returning an empty string here? Importantly, I think returning an empty string isn't actually the right thing to do, no matter how likely it is.

Instead, our return type should be a string optional: String? and we should return nil when something fails.

That way, it's easier on the end user to be certain this returned a valid value:

if let ssid = UIDevice.currentDevice().SSID {
    // do stuff
} else {
    // error fetching ssid
}

This seems easier than the alternative:

let ssid = UIDevice.currentDevice().SSID
if countElements(ssid) > 0 {
    // do stuff
} else {
    // error fetching ssid
}

So, with that out of the way, the only problem I have here is all of this unnecessary nesting and unnecessary variables:

public var SSID: String? {
    get {
        if let interfaces = CNCopySupportedInterfaces() {
            let interfacesArray = interfaces.takeRetainedValue() as [String]
            if let unsafeInterfaceData = CNCopyCurrentNetworkInfo(interfacesArray.firstObject) {
                let interfaceData = unsafeInterfaceData.takeRetainedValue() as Dictionary
                return interfaceData["SSID"]
            }
        }
        return nil
    }
}

Ultimately, there's no reason here to have the three layers of else blocks that all actually do nothing. In your code you initialize currentSSID to an empty string, and then only change it to anything else if you can get the SSID. Your three else blocks change currentSSID from an empty string to an empty string, so just omit them completely.

\$\endgroup\$
1
  • \$\begingroup\$ I find all your comments very valuable but I think there are some issues in your last piece of code. For instance firstObject doesn't exist in Swift and there should be a conditional cast on the value returned (return interfaceData["SSID"] as? String) \$\endgroup\$
    – Alexis
    Jan 28, 2015 at 10:28
7
\$\begingroup\$

First spacing; a blank line between each line of code is distracting.

second each if introduces a new nesting level instead invert the conditino and use early returns:

class func getSSID() -> String {

    let interfaces = CNCopySupportedInterfaces()
    if interfaces == nil {
        return ""
    }

    let interfacesArray = interfaces.takeRetainedValue() as [String]
    if interfacesArray.count <= 0 {
        return ""
    }

    let interfaceName = interfacesArray[0] as String
    let unsafeInterfaceData = CNCopyCurrentNetworkInfo(interfaceName)
    if unsafeInterfaceData == nil {
        return ""
    }

    let interfaceData = unsafeInterfaceData.takeRetainedValue() as Dictionary!

    return interfaceData["SSID"] as String
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.