Makes objects function more swifty.#74
Makes objects function more swifty.#74valeriyvan wants to merge 2 commits intoInstagram:masterfrom valeriyvan:swift
Conversation
|
@jessesquires had some comments on the last swifty PR we did after I merged it, gonna let him take a quick look at this before we import. |
| } | ||
| } | ||
| return items | ||
| guard selectedClass != nil else { return data.map { $0 as! IGListDiffable } } |
There was a problem hiding this comment.
nit: I think this would be easier to read if we put the return on a newline and the final } on a newline
There was a problem hiding this comment.
Agree, else clause of guard statement should be on separate line if it has something substantially longer of simple return.
| } | ||
| return items | ||
| guard selectedClass != nil else { return data.map { $0 as! IGListDiffable } } | ||
| return data.filter { type(of: $0) == selectedClass! }.map { $0 as! IGListDiffable } |
There was a problem hiding this comment.
actually, now i remember the issues I saw with this. scratch that first comment. 😄
the previous version combined the 2 conditions in a single if. Now this is broken up and harder to follow, not to mention we've written .map { $0 as! IGListDiffable } twice. 😊
why can't the initial filter contain both conditions? couldn't this be something like this?
return data.filter {
if let theClass = selectedClass {
return type(of: $0) == theClass
}
return false
}.map { $0 as! IGListDiffable }I think this is more readable + more concise.
There was a problem hiding this comment.
Sure, initial filter could have both conditions.
But it hides that if selectedClass is nil, function returns just it's input array with transformed elements.
Otherwise it filters input array and then transforms it's elements.
Version I am proposing keeps that clear.
Less condensed but more clear.
|
@valeriyvan updated the pull request - view changes |
|
Thanks for importing. If you are a Facebook employee, you can view this diff on Phabricator. |
4 participants
Changes in this pull request
Makes objects function more swifty.
Pull request checklist