4
\$\begingroup\$

I have some coordinates looking like

N47° 15' 36.75",E011° 20' 38.28",+001906.00

and I've created a class to parse and convert them to Double:

struct PLNWaypointCoordinate {
    var latitude: Double = 0.0
    var longitude: Double = 0.0

    init(coordinateString: String) {
        self.latitude = convertCoordinate(string: coordinateString.components(separatedBy: ",")[0])
        self.longitude = convertCoordinate(string: coordinateString.components(separatedBy: ",")[1])
    }

    private func convertCoordinate(string: String) -> Double {
        var separatedCoordinate = string.characters.split(separator: " ").map(String.init)

        let direction = separatedCoordinate[0].components(separatedBy: CharacterSet.letters.inverted).first
        let degrees = Double(separatedCoordinate[0].components(separatedBy: CharacterSet.decimalDigits.inverted)[1])
        let minutes = Double(separatedCoordinate[1].components(separatedBy: CharacterSet.decimalDigits.inverted)[0])
        let seconds = Double(separatedCoordinate[2].components(separatedBy: CharacterSet.decimalDigits.inverted)[0])

        return convert(degrees: degrees!, minutes: minutes!, seconds: seconds!, direction: direction!)
}

    private func convert(degrees: Double, minutes: Double, seconds: Double, direction: String) -> Double {
        let sign = (direction == "W" || direction == "S") ? -1.0 : 1.0
        return (degrees + (minutes + seconds/60.0)/60.0) * sign
    }

}

Is there a better and safer way to perform this conversion?

Last method I've picked up here. Sorry, but I can't find the link to reference it.

\$\endgroup\$
4
\$\begingroup\$

You have defined a value type (struct) and not a class, which is good. In

var latitude: Double = 0.0
var longitude: Double = 0.0

you can omit the type annotations because the compiler can infer the type automatically:

var latitude = 0.0
var longitude = 0.0

But actually I would go the other way around and replace the initial values by an init() method. The reason is that – since you defined your own init method – there is no default memberwise initializer anymore. Therefore I would start with

struct PLNWaypointCoordinate {
    var latitude: Double
    var longitude: Double

    init(latitude: Double, longitude: Double) {
        self.latitude = latitude
        self.longitude = longitude
    }

    // ...
}

which allows you to create a value not only from a string, but also from a given latitude and longitude:

let c = PLNWaypointCoordinate(latitude: ..., longitude: ...)

Your init(coordinateString: String) method assumes that the given string is in a valid format and can crash otherwise (by accesssing out-of-bounds array elements or unwrapping nils). As @Ashley already said in his answer, you should define a failable initializer instead, which returns nil if the input is invalid:

init?(coordinateString: String) { ... }

Alternatively, define a throwing initializer:

init(coordinateString: String) throws { ... }

which throws an error for invalid input.

Your helper methods convertCoordinate and convert are private, which is good. They do not use any properties of the value, which means that they can be made static.

The conversion helper function is very lenient, it will for example accept X47👺 15👺 36.75👺 instead of N47° 15' 36.75" as latitude. It does not check that the coordinate starts with a valid direction, or that the proper separators are used.

I don't know what the final part +001906.00 stands for, but that is ignored completely in your code.

There is also a flaw in your conversion, the fractional part of the seconds is ignored, e.g. 36.75" is taken as 36 seconds.

I would suggest to use Scanner instead, which makes it relatively easy to check for a valid input and simultaneously parse the values into variables. To distinguish between latitude and longitude, two arguments for the positive and the negative direction are passed to the helper method.

The complete code then looks like this:

struct PLNWaypointCoordinate {
    var latitude: Double
    var longitude: Double

    init(latitude: Double, longitude: Double) {
        self.latitude = latitude
        self.longitude = longitude
    }

    init?(coordinateString: String) {
        let components = coordinateString.components(separatedBy: ",")
        guard components.count >= 2,
            let latitude = PLNWaypointCoordinate.convertCoordinate(coordinate: components[0],
                                                                   positiveDirection: "N",
                                                                   negativeDirection: "S"),
            let longitude = PLNWaypointCoordinate.convertCoordinate(coordinate: components[1],
                                                                    positiveDirection: "E",
                                                                    negativeDirection: "W")
            else {
                return nil
        }
        self.init(latitude: latitude, longitude: longitude)
    }

    private static func convertCoordinate(coordinate: String,
                                          positiveDirection: String,
                                          negativeDirection: String) -> Double? {
        // Determine the sign from the first character:
        let sign: Double
        let scanner = Scanner(string: coordinate)
        if scanner.scanString(positiveDirection, into: nil) {
            sign = 1.0
        } else if scanner.scanString(negativeDirection, into: nil) {
            sign = -1.0
        } else {
            return nil
        }

        // Parse degrees, minutes, seconds:
        var degrees = 0
        var minutes = 0
        var seconds = 0.0
        guard scanner.scanInt(&degrees),         // Degrees (integer),
            scanner.scanString("°", into: nil),  // followed by °,
            scanner.scanInt(&minutes),           // minutes (integer)
            scanner.scanString("'", into: nil),  // followed by '
            scanner.scanDouble(&seconds),        // seconds (floating point),
            scanner.scanString("\"", into: nil), // followed by ",
            scanner.isAtEnd                      // and nothing else.
            else { return nil }

        return sign * (Double(degrees) + Double(minutes)/60.0 + seconds/3600.0)
    }
}
\$\endgroup\$
  • \$\begingroup\$ Scanner is exactly the way to handle this - great answer \$\endgroup\$ – Ashley Mills Jan 23 '17 at 10:37
  • \$\begingroup\$ @AshleyMills: Thank you! – But you covered the most imported point already (making the initialization failable). \$\endgroup\$ – Martin R Jan 23 '17 at 12:12
2
\$\begingroup\$

You should certainly be removing any force unwraps - which in turn means you'll need to use more optional values…

First thoughts

struct PLNWaypointCoordinate: CustomStringConvertible {
    var latitude: Double = 0.0
    var longitude: Double = 0.0

    init?(coordinateString: String) {
        let components = coordinateString.components(separatedBy: ",")
        guard components.count >= 2,
            let latitude = convertCoordinate(string: components[0]),
            let longitude = convertCoordinate(string: components[1])  else {
                return nil
        }

        self.latitude = latitude
        self.longitude = longitude
    }

    var description: String {
        return "(\(latitude),\(longitude))"
    }

    private enum Direction: String {
        case N, E, S, W
        var sign: Double { return self == .S || self == .W ? -1 : 1 }
    }
    private func convertCoordinate(string: String) -> Double? {
        let separatedCoordinate = string.characters.split(separator: " ").map(String.init)

        let nonAlphas = CharacterSet.letters.inverted
        let nonDecimals = CharacterSet.decimalDigits.inverted
        guard let directionString = separatedCoordinate[0].components(separatedBy: nonAlphas).first,
            let direction = Direction(rawValue: directionString),
            let degrees = Double(separatedCoordinate[0].components(separatedBy: nonDecimals)[1]),
            let minutes = Double(separatedCoordinate[1].components(separatedBy: nonDecimals)[0]),
            let seconds = Double(separatedCoordinate[2].components(separatedBy: nonDecimals)[0]) else {
                return nil
        }

        return convert(degrees: degrees, minutes: minutes, seconds: seconds, direction: direction)
    }

    private func convert(degrees: Double, minutes: Double, seconds: Double, direction: Direction) -> Double {
        return (degrees + (minutes + seconds/60)/60) * direction.sign
    }
}
\$\endgroup\$
  • \$\begingroup\$ Thank you! I was planning on cleaning up the force unwraps as what I posted it was just a prototype, but I didn't want to start the cleanup until I got some more input. \$\endgroup\$ – Alex Bartiş Jan 22 '17 at 15:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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