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.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jan 22, 2017 at 12:26
\$\endgroup\$
0

2 Answers 2

5
\$\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)
 }
}
answered Jan 22, 2017 at 14:31
\$\endgroup\$
2
  • \$\begingroup\$ Scanner is exactly the way to handle this - great answer \$\endgroup\$ Commented Jan 23, 2017 at 10:37
  • \$\begingroup\$ @AshleyMills: Thank you! – But you covered the most imported point already (making the initialization failable). \$\endgroup\$ Commented Jan 23, 2017 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
 }
}
answered Jan 22, 2017 at 13:05
\$\endgroup\$
1
  • \$\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\$ Commented Jan 22, 2017 at 15:06

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.