patternswiftMinor
Safe init using failable guarded initializer
Viewed 0 times
initusingfailablesafeguardedinitializer
Problem
I am wondering if that is a safe and proper init, with one failable init guarded and an init.
Are the force-unwrap correct in the
Are the force-unwrap correct in the
self.init as I know they are those types ?struct Item: Equatable {
let id: NSNumber
let displayName: String
let amount: NSNumber
init(id:NSNumber, displayName:String, amount:NSNumber) {
self.id = id
self.displayName = displayName
self.amount = amount
}
init?(dictionnary:NSDictionary) {
guard let id = dictionnary["id"] , id is NSNumber else {
return nil
}
guard let displayName = dictionnary["display_name"] , displayName is String else {
return nil
}
guard let amount = dictionnary["amount"] , amount is NSNumber else {
return nil
}
self.init(id:id as! NSNumber, displayName: displayName as! String, amount: amount as! NSNumber)
}
}
func ==(lhs: Item, rhs: Item) -> Bool {
return lhs.id == rhs.id
}Solution
The separation into a non-failable and a failable init method is
fine.
The forced casts with
that the three variables
respective types
However:
Forced casts should (and can!) be avoided in most cases, in the
same way as forced unwrappings. The program would be terminated
immediately if the cast fails.
In your code that is quite easy,
by replacing the type checks
The latter return a value of the type
cast unnecessary. In addition, it simplifies and shortens the code
of the failable init method:
As a further simplification, combine the optional bindings into
a single
reminded my in a comment):
Some other points:
fine.
The forced casts with
as! are safe in your code, because it is verified beforethat the three variables
id, displayName, number have therespective types
Number, String and NSNumber.However:
- The code is error-prone: Changes to the types must be done at two places.
- It is not immediately obvious that it works correctly.
Forced casts should (and can!) be avoided in most cases, in the
same way as forced unwrappings. The program would be terminated
immediately if the cast fails.
In your code that is quite easy,
by replacing the type checks
is T by conditional casts as? T.The latter return a value of the type
T and that makes the forcedcast unnecessary. In addition, it simplifies and shortens the code
of the failable init method:
init?(dictionary: NSDictionary) {
guard let id = dictionary["id"] as? NSNumber else {
return nil
}
guard let displayName = dictionary["display_name"] as? String else {
return nil
}
guard let amount = dictionary["amount"] as? NSNumber else {
return nil
}
self.init(id: id, displayName: displayName, amount: amount)
}As a further simplification, combine the optional bindings into
a single
guard statement (attribution goes to @Knight0fDragon whoreminded my in a comment):
init?(dictionary: NSDictionary) {
guard
let id = dictionary["id"] as? NSNumber,
let displayName = dictionary["display_name"] as? String,
let amount = dictionary["amount"] as? NSNumber else {
return nil
}
self.init(id: id, displayName: displayName, amount: amount)
}Some other points:
- Your spacing is not consistent.
- The indenting is totally wrong.
- The correct spelling is
dictionary.
Code Snippets
init?(dictionary: NSDictionary) {
guard let id = dictionary["id"] as? NSNumber else {
return nil
}
guard let displayName = dictionary["display_name"] as? String else {
return nil
}
guard let amount = dictionary["amount"] as? NSNumber else {
return nil
}
self.init(id: id, displayName: displayName, amount: amount)
}init?(dictionary: NSDictionary) {
guard
let id = dictionary["id"] as? NSNumber,
let displayName = dictionary["display_name"] as? String,
let amount = dictionary["amount"] as? NSNumber else {
return nil
}
self.init(id: id, displayName: displayName, amount: amount)
}Context
StackExchange Code Review Q#138460, answer score: 6
Revisions (0)
No revisions yet.