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.
2 Answers 2
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 nil
s). 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 throw
ing 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(°rees), // 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)
}
}
-
\$\begingroup\$ Scanner is exactly the way to handle this - great answer \$\endgroup\$Ashley Mills– Ashley Mills2017年01月23日 10:37:52 +00:00Commented Jan 23, 2017 at 10:37
-
\$\begingroup\$ @AshleyMills: Thank you! – But you covered the most imported point already (making the initialization failable). \$\endgroup\$Martin R– Martin R2017年01月23日 12:12:59 +00:00Commented Jan 23, 2017 at 12:12
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
}
}
-
\$\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\$trusk– trusk2017年01月22日 15:06:24 +00:00Commented Jan 22, 2017 at 15:06