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
 }
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jan 22, 2015 at 14:52
\$\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.

answered Jan 22, 2015 at 21:25
\$\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\$ Commented 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
}
answered Jan 22, 2015 at 15:10
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.