Skip to content
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

Avoid testing floats for equality in PhotoViewController #135

Merged
merged 2 commits into from
Jan 26, 2016

Conversation

cdzombak
Copy link
Contributor

closes #133

@cdzombak
Copy link
Contributor Author

@skyline75489, can you verify whether this approach fixes the issue you reported in #133? If so, I'd prefer this to the two-step comparison you outlined in that issue; this approach seems easier to read.

@cdzombak cdzombak modified the milestone: 1.0.1 Jan 19, 2016
if (self.scalingImageView.zoomScale >= self.scalingImageView.maximumZoomScale) {

if ((self.scalingImageView.zoomScale - self.scalingImageView.maximumZoomScale) >= 0.0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks fine. However commenting out the if clause entirely causes no unit test failures. Expand the test coverage to include this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, but I don't think we have any test coverage for the UIKit stuff in general so that becomes a much bigger task

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdzombak I don't think this works. I've been in a situation where self.scalingImageView.zoomScale is slightly smaller than self.scalingImageView.maximumZoomScale(well, not really small but strict-float-comparison small). That's why I add the ABS to deal with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skyline75489 ah, that makes sense. thanks!

@cdzombak cdzombak self-assigned this Jan 19, 2016
@soundsokay
Copy link
Contributor

👍

cdzombak added a commit that referenced this pull request Jan 26, 2016
Avoid testing floats for equality in PhotoViewController
@cdzombak cdzombak merged commit 3f944c2 into develop Jan 26, 2016
@cdzombak cdzombak deleted the cdz/133-float-comparisons branch January 26, 2016 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants