KVO observer isn't removed on removeFromSuperview.

[sunshinejr]

So for now we are adding KVO observer in init, and deallocating in deinit. This isn't the best approach since we could want to remove the view for a while and add it again, which in fact can't be done because we still have the observer on.

One way to fix this issue is to add an observer on willMoveToSuperview(newSuperview:) and remove it on removeFromSuperview(), but I am not 100% sure that it will works every time. Any opinions?

[frranck]

@sunshinejr What if you skipped the KVO and used scrollViewDidScroll: ?

http://khanlou.com/2013/12/kvo-considered-harmful/

[sunshinejr]

We've had it in version 0.1 and wasn't as smooth with offset changing. 😥 KVO made it pretty easy. Also KVO is a danger for apps because you can observe one property from many controllers/objects and it's really hard to test from where it came from. In our case we use only one observing in one file, so it isn't really a problem in our case.

[aocenas]

I am not sure what the problem is with the init/deinit approach. Removing carousel from view hierarchy should not be a problem regardless of the KVO observer, or is it? Therefore, if we remove the view from the view hierarchy, there shouldn't be any scroll events and so keeping the observer reference shouldn't do much harm. Or am I missing something in here?

[frranck]

@aocenas the removeView does this:

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'An instance 0x7a233400 of class UIScrollView was deallocated while key value observers were still registered with it. Current observation info: <NSKeyValueObservationInfo 0x79976ca0> (
<NSKeyValueObservance 0x7994f940: Observer: 0x79969de0, Key path: contentOffset, Options: <New: YES, Old: YES, Prior: NO> Context: 0x0, Property: 0x7995d990>
)'
*** First throw call stack:
(
    0   CoreFoundation                      0x0084ea14 __exceptionPreprocess + 180
    1   libobjc.A.dylib                     0x02942e02 objc_exception_throw + 50
    2   CoreFoundation                      0x0084e93d +[NSException raise:format:] + 141
    3   Foundation                          0x00e4db57 NSKVODeallocate + 353
    4   UIKit                               0x01d3b13d -[UIView(UIKitManual) release] + 142
    5   libobjc.A.dylib                     0x0295534f objc_release + 47
    6   libobjc.A.dylib                     0x0295652d _ZN12_GLOBAL__N_119AutoreleasePoolPage3popEPv + 371
    7   CoreFoundation                      0x00725508 _CFAutoreleasePoolPop + 24
    8   CoreFoundation                      0x0075dcdc __CFRunLoopRun + 2364
    9   CoreFoundation                      0x0075d0e6 CFRunLoopRunSpecific + 470
    10  CoreFoundation                      0x0075cefb CFRunLoopRunInMode + 123
    11  GraphicsServices                    0x077df664 GSEventRunModal + 192
    12  GraphicsServices                    0x077df4a1 GSEventRun + 104
    13  UIKit                               0x01472bfa UIApplicationMain + 160
    14  carer                               0x00117ffc main + 140
    15  libdyld.dylib                       0x04129a21 start + 1

[aocenas]

@frranck Hmm if I remember correctly this is exactly the bug I was trying to solve with this commit: https://github.com/DroidsOnRoids/SwiftCarousel/pull/4/commits/e62cdd5bf72e8dd94881c29f0242fbc557dba3ff. Are you sure you are not on some older version?

[frranck]

@aocenas yes I have your change in my version, it's SwiftCarousel (0.3.4)

[aocenas]

I will have to check the removeFromSuperview case, I only tested case when whole view hierarchy was deallocated together with its viewController.

[frranck]

@aocenas I'm doing a carouselView.removeFromSuperview() with var carouselView: SwiftCarousel!

[frranck]

@aocenas I tried with SwiftCarousel (0.5) and still have the problem