-
-
Save radex/6c4f8da03ba57422574b to your computer and use it in GitHub Desktop.
// Option 1: if..elseif + optional unwrapping conditions | |
private func handleNotificationAction(id: String?, userInfo: [NSObject: AnyObject], responseInfo: [NSObject: AnyObject]?, completion: () -> Void) { | |
let taskId = userInfo["extra"]?["task_hash"] as? String | |
let projectId = userInfo["extra"]?["project_hash"] as? String | |
let comment = responseInfo?["UIUserNotificationActionResponseTypedTextKey"] as? String | |
if id == "comment", let task = taskId, comment = comment where !comment.isEmpty { | |
} else if id == "invitation_accept", let project = projectId { | |
} else if id == "invitation_decline", let project = projectId { | |
} else if id == "star", let task = taskId { | |
} else if id == "complete", let task = taskId { | |
} else if id == "show" || id == "comment" || id == "delegate", let task = taskId { | |
} else { | |
// assertionFailure | |
} | |
} | |
// Option 2: switch + force unwrap | |
private func handleNotificationAction(id: String?, userInfo: [NSObject: AnyObject], responseInfo: [NSObject: AnyObject]?, completion: () -> Void) { | |
let taskId = userInfo["extra"]?["task_hash"] as? String | |
let projectId = userInfo["extra"]?["project_hash"] as? String | |
let comment = responseInfo?["UIUserNotificationActionResponseTypedTextKey"] as? String | |
switch id { | |
case "comment"? where taskId != nil && comment != nil && !comment.isEmpty: | |
case "invitation_accept"? where projectId != nil: | |
case "invitation_decline"? where projectId != nil: | |
case "star"? where taskId != nil: | |
case "complete"? where taskId != nil: | |
case "show"?, "comment"?, "delegate"? where taskId != nil: | |
default: | |
// assertionFailure | |
} | |
} |
How about like this:
switch id {
case "comment":
if let comment = comment {
}
case "star":
if let taskId = taskId {
}
}
BTW, why not add a guard for the id
at the beginning to unwrap it? (Or don't accept an optional in the first place...)
@mackuba The problem with nesting if let
inside case
, aside from the fact that it's noisy is that I'll need fallthrough
in the else case, so that it gets to default
where I want to raise an error.
Same with the id
unwrapping — if it's nil, I want to make an assertion failure.
Here is a solution without force unwraps ....
switch (id, taskId, projectId, comment) {
case ("comment"?, let taskId?, _, let comment?) where comment.isEmpty == false:
break
case ("invitation_accept"?, _, let projectId?, _):
break
...
default:
break
}
Agree with @josephlord (even though he's missing ?s at the end of the patterns), or otherwise define your enum Notification
with the right associated values and give it an init?(id: String, taskId: String? = nil, projectId: String? = nil, comment: String? = nil)
@DeFrenZ I could make an enum, sure, but this doesn't remove the ugliness of a switch/if..else, just encapsulates it somewhere. This is about the only place where I'm interpreting the notification JSON, so I don't really see the point of abstracting notifications...
the only point would be to separate the "validation" and "presentation" logic of the notification handling. Not too much since it's used only in one place, but also makes it easier if you want to then make objects manage those by themselves and you can pass them around.
But anyway, at the "validation" step, I would go with a tuples switch
. The correct syntax for the previous one (with comment
as a tuple element as well):
private func handleNotificationAction(id: String?, userInfo: [NSObject: AnyObject], responseInfo: [NSObject: AnyObject]?, completion: () -> Void) {
let customPayload = userInfo["extra"] as? [String: AnyObject]
let taskId = customPayload?["task_hash"] as? String
let projectId = customPayload?["project_hash"] as? String
let comment = responseInfo?["UIUserNotificationActionResponseTypedTextKey"] as? String
switch (id, taskId, projectId, comment) {
case ("comment"?, _?, _, let comment?) where !comment.isEmpty:
break
case ("invitation_accept"?, _, _?, _):
break
case ("invitation_decline"?, _, _?, _):
break
case ("star"?, _?, _, _):
break
case ("complete"?, _?, _, _):
break
case (let otherId?, _?, _, _) where ["show", "comment", "delegate"].contains(otherId):
break
default:
return // assertionFailure
}
}
and you can substitute some _?
with let
s if needed
Late to the party, but I would split it into two parts: converting arguments to some sane structure (e.g. an enum), and then handling it.
private enum NotificationAction {
case Comment(taskId: String, text: String?)
case Star(taskId: String)
case Complete(taskId: String)
case Show(taskId: String)
case Delegate(taskId: String)
case InvitationAccept(projectId: String)
case InvitationDecline(projectId: String)
init?(actionId: String, taskId: String?, projectId: String?, commentText: String?) {
switch (taskId, projectId) {
case let (taskId?, nil): switch actionId {
case "comment": self = Comment(taskId: taskId, text: commentText.flatMap { $0.isEmpty ? nil : $0 })
case "star": self = Star(taskId: taskId)
case "complete": self = Complete(taskId: taskId)
case "show": self = Show(taskId: taskId)
case "Delegate": self = Delegate(taskId: taskId)
default: return nil
}
case let (nil, projectId?): switch actionId {
case "invitation_accept": self = InvitationAccept(projectId: projectId)
case "invitation_decline": self = InvitationDecline(projectId: projectId)
default: return nil
}
default: return nil
}
}
}
private func handleNotificationAction(id: String?, userInfo: [NSObject: AnyObject], responseInfo: [NSObject: AnyObject]?, completion: () -> Void) {
let taskId = userInfo["extra"]?["task_hash"] as? String
let projectId = userInfo["extra"]?["project_hash"] as? String
let comment = responseInfo?["UIUserNotificationActionResponseTypedTextKey"] as? String
guard let
id = id,
action = NotificationAction(actionId: id, taskId: taskId, projectId: projectId, commentText: comment)
else {
assertionFailure()
completion()
return
}
switch action {
// ...
}
}
Oh, a slight improvement over the previous one:
func ~= <E: Equatable, C: CollectionType where E == C.Generator.Element> (lhs: C, rhs: E) -> Bool {
return lhs.contains(rhs)
}
private func handleNotificationAction(id: String?, userInfo: [NSObject: AnyObject], responseInfo: [NSObject: AnyObject]?, completion: () -> Void) {
let customPayload = userInfo["extra"] as? [String: AnyObject]
let taskId = customPayload?["task_hash"] as? String
let projectId = customPayload?["project_hash"] as? String
let comment = responseInfo?["UIUserNotificationActionResponseTypedTextKey"] as? String
switch (id, taskId, projectId, comment) {
case ("comment"?, _?, _, let comment?) where !comment.isEmpty:
break
case ("invitation_accept"?, _, _?, _):
break
case ("invitation_decline"?, _, _?, _):
break
case ("star"?, _?, _, _):
break
case ("complete"?, _?, _, _):
break
case (["show", "comment", "delegate"]?, _?, _, _):
break
default:
return // assertionFailure
}
}
...and it works! I think that matching can be a good addition to the standard extensions :P
How about going completely left field and doing something like the following:
typealias NotifyActionSuggestionHandler = (taskId : String?, projectId : String?, comment: String?, completion: () -> Void) -> ()
func handleComment(taskId taskId: String?, projectId : String?, comment: String?, completion: () -> Void) {
guard let tid = taskId, let c = comment where !c.isEmpty else {
print("Comment Data Error")
return
}
print("Comment: task(\(tid))")
completion()
}
func handleInvitationAccept(taskId taskId: String?, projectId : String?, comment: String?, completion: () -> Void) {
print("Invitation Accept")
completion()
}
var handleStar : NotifyActionSuggestionHandler = {(tid,pid,com,comp) in
print("Handle Star")
comp()
}
let commands = ["comment" : handleComment, "invitation_accept" : handleInvitationAccept, "star" : handleStar]
func handleNotificationActionSuggestion(id: String?, userInfo: [NSObject: AnyObject], responseInfo: [NSObject: AnyObject]?, completion: () -> Void) {
guard let actionId = id, let action = commands[actionId] else {return} // Or assertion failure (fail fast)
action(taskId: userInfo["extra"]?["task_hash"] as? String, projectId: userInfo["extra"]?["project_hash"] as? String, comment: responseInfo?["UIUserNotificationActionResponseTypedTextKey"] as? String, completion: completion)
}
let user : [NSObject:AnyObject] = [:]
let response : [NSObject:AnyObject] = [:]
handleNotificationActionSuggestion("comment", userInfo: user, responseInfo: response, completion: {print("Comment Done")})
handleNotificationActionSuggestion("invitation_accept", userInfo: user, responseInfo: response, completion: {print("Invitation Done")})
handleNotificationActionSuggestion("star", userInfo: user, responseInfo: response, completion: {print("Star Done")})
Keep functions small, easily extendible and avoids complex conditionals/switch on strings - also brings testing benefits. (Although I do like narfdotpl suggestion)
How about switching on a tuple.