Use of super in categories

[tyrone-sudeium]

I'm pretty concerned by the use of:

- (void)touchesBegan:(NSSet *)touches withEvent:(UIEvent *)event {
    [super touchesBegan:touches withEvent:event];
    BKTouchBlock block = [self associatedValueForKey:&kViewTouchDownBlockKey];
    if (block)
        block(touches, event);
}

In the UIView (BlocksKit) category. Just making sure everyone's aware that because this is in a category, super will send the message to UIView's superclass: UIResponder, and not to the original implementation of UIView's -touchesBegan:withEvent:. This is a problem because UIView might be doing important stuff in its -touchesBegan:withEvent: that could get completely bypassed by your implementation!

I'm actually surprised clang doesn't freak out with a Category is implementing a method which will also be implemented by its primary class warning - I guess it's because it's UIResponder that implements the original method...

In order to invoke the original implementation of -touchesBegan:withEvent:, you need to use Method Swizzling which has its own huge set of problems (not the least of which is the unpredictability when used with other libraries).

I bring this up because I'm getting an unpredictable, random crash in my current application under iOS 6 in the simulator, where upon touching a UI element that results in a navigation controller push/pop, the runtime will inexplicably crash sometime shortly after in the same runloop cycle with a __sel_registerName EXC_BAD_ACCESS. I can't prove that it's definitely BlocksKit right now, but when I unlink BlocksKit, I no longer see the issue. It only happens under very specific conditions and it's very difficult to replicate reliably.

Can someone tell me if I'm way off base here?

[zwaldowski]

This is a valid fear. The actual behavior of the call has depended on the compiler at times. In the future I suppose I'll use the runtime to get the actual/real/current super-interface and attach the BlocksKit one via swizzling. Swizzling isn't actually that scary if you're careful and make sure to call the original.

[Pretz]

After looking at this closely, this concerns me deeply too, and makes me distrustful of using BlocksKit for a production application. In particular, the documentation in UIResponder says the following:

The default implementation of this method does nothing. However immediate UIKit subclasses of UIResponder, particularly UIView, forward the message up the responder chain. To forward the message to the next responder, send the message to super (the superclass implementation); do not send the message directly to the next responder.

By overriding this method in a category and calling super directly on UIResponder, you are likely breaking the propagation of touch events up the responder chain as implemented in UIView. The only way to fix this is to swizzle these methods instead of overriding them. I wouldn't be surprised if @tyrone-sudeium's issue was because the UIView implementation is receiving a touchesCancelled:withEvent: call (which isn't overridden by BlocksKit), without any matching touchesBegan:withEvent:.

I'd like to request that with or without swizzling this feature be included as optionally compiled. If issues with other libraries arise, a simple #undef UIVIEW_BLOCKS would fix it for folks who wanted to use the other features of BlocksKit.

[zwaldowski]

Looking into it.

[getaaron]

You can do it without method swizzling: http://www.cocoawithlove.com/2008/03/supersequent-implementation.html

[Pretz]

Conclusion

Yes, you can invoke the default class implementation from a category on that class. In most cases though, since it is slower at runtime than method swizzling, it is primarily only useful for debugging where 5 lines less code makes it much easier to use.

The speed at runtime is slower than method swizzling because it bypasses cached method lookup. Where cached method lookup takes about 5 nanoseconds on newer machines, this approach takes about 5 microseconds on typical objects with around 50 methods in their hierarchy (slightly slower than uncached method lookup) and like uncached method lookup, it can take longer for classes with very large method tables.

[zwaldowski]

Okay. Interesting information. It might not be suitable in that case to
override the default implementation in that case, considering you'll have
hundreds of views in a typical app. We're looking into it, the best method
for this might be not to do anything at all, unfortunately (at least as far
as our current implementation goes). Or maybe some KVO-like subclassing so
we actually do benefit from super-calling, cached-methods, etc.

[seivan]

This seems only to be an issue as the you're not really 100% certain that the default implementation doesn't do anything. Right? I mean if the documentation outright said that the default implementation doesn't do jack, then it wouldn't be a concern?

[Pretz]

@seivan The documentation for UIResponder specifically says that UIView does have a default implementation and recommends calling it in certain cases.

[seivan]

@Pretz Yes, I know. I didn't mean this particular instance specifically, just asking generally :-)

I am just talking general here, if the documentation outright says that the default implementation does nothing then it would be OK to do this, instead of hijacking the original message call, or is that a gray area?

I'm mostly referencing @a2's comment
where @a2 brought up prepareForSegue: -> where the default implementation does nothing (according to docs), and there is no chain of events or bubbled messages.

[zwaldowski]

In the era of UIGestureRecognizer, I'm leaning towards the idea of removing the touch blocks on UIView in the next version of BlocksKit. If you have what you think is a valid use case for these, I encourage you to speak up.

[seivan]

Are you suggesting to retire the touch blocks from UIView and to rely on UIGestureRecognizer?

[zwaldowski]

My intent is to get rid of bk_onTouchDownBlock, bk_onTouchMoveBlock, and bk_onTouchUpBlock, yes.

[avalanched]

Why wasn't this included in the 1.8.3 release? i really need it.

[a2]

It will be included in the 2.0.0 release. All in good time.