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.