-
Notifications
You must be signed in to change notification settings - Fork 806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Protecting UIButton's CGSize calculations against nil receivers; more… #2094
Conversation
… fallout from microsoft#1965 and microsoft#1365. Fixes microsoft#2066.
// TODO: Should we be asking our label for this? | ||
NSString* currentTitle = self.currentTitle; | ||
CGSize titleSize = CGSizeZero; | ||
if (currentTitle) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currentTitle [](start = 8, length = 12)
[currentTitle length] > 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure, and i think the safest approach is to stick to functional equivalence just to be safe
In reply to: 103035505 [](ancestors = 103035505)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying we should not do the same check like below: currentTitle && currentTitle.length > 0?
In reply to: 103035505 [](ancestors = 103035505)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm saying, 'i don't know'; do we have tests to validate this behavior? I just went through and kept the existing functionality. The other functions check the length, and this one doesn't, because that's how it worked previously.
In reply to: 103035705 [](ancestors = 103035705,103035505)
// TODO: Should we be asking our label for this? | ||
NSString* currentTitle = self.currentTitle; | ||
CGSize titleSize = CGSizeZero; | ||
if (currentTitle && currentTitle.length > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per oliver's comment; nil.length
is 0, so this is a redundant check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know, but if we don't leave the explicit nil check, someone may forget next time they come through here?
In reply to: 103035907 [](ancestors = 103035907)
… fallout from microsoft#1965 and microsoft#1365. Fixes microsoft#2066. (microsoft#2094)
… fallout from #1965 and #1365. Fixes #2066.