zoomRelativePoint: Can Lead to NaN Values

[ashfurrow]

Consider the following code:

- (CGPoint)zoomRelativePoint:(CGPoint)point
{
    CGFloat x = (self.contentSize.width / self.originalSize.width) * point.x;
    CGFloat y = (self.contentSize.height / self.originalSize.height) * point.y;
    return CGPointMake(round(x), round(y));
}

If originalSize hasn't been set from CGPointZero, then the resulting x and y values can be NaN, so if you rely on the return value for this for some view geometry, it can lead to an app crash.

[ashfurrow]

So I'm thinking that returning CGPointZero would work well as a sentinel value? Or should we do something else, like return {-1, -1}? Gosh I can't wait for Swift.

[orta]

CGPointZero, think its better to not throw everything out of whack.

[ashfurrow]

👍

[dblock]

I think CGPointZero would be incorrect, it's the geometric zero. If you cannot do the math because you don't have a size you should probably explicitly fail. Raise an error?

[ashfurrow]

Yeah unfortunately there's no equivalent of CGRectNull here. Maybe this should be something that the calling code is responsible for checking against?

[dblock]

Maybe just assert and return the original point otherwise?

[ashfurrow]

Not sure if I like that, though. I mean, that'd bring down the whole app just for the originalSize being zero in either dimension, which doesn't really make any sense, is completely valid from a UIKit-perspective. What'd be awesome is if we had optional types (heh), but we don't. The "best" approach I can think of would be to wrap the point in a struct of our own with a "valid" flag included. But that seems like overkill.

[dblock]

Agreed on overkill. Do you think adding an assert and not changing anything in the code itself would be sufficient? A store app will run through the assert and do what the current version of the library is doing (NaN).

[ashfurrow]

Yeah I think you're right @dblock – probably the best compromise.